Conversation
There was a problem hiding this comment.
Pull request overview
Introduces the initial WinUI 3 packaged desktop port of WinIRC alongside a new shared WinIRC.Core library, keeping the legacy UWP app in the solution for reference while migrating runtime/session/storage logic into the shared layer.
Changes:
- Add
WinIRC.WinUI(packaged WinUI 3) with shell UI, connect/settings views, detached windows, and supporting services. - Add
WinIRC.Coreshared models/interfaces plus IRC runtime + workspace/session management. - Add JSON-backed settings/server persistence and best-effort migration from legacy package state; update solution + docs.
Reviewed changes
Copilot reviewed 79 out of 88 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| WinIRC/WinIRC.csproj | Bumps Windows SDK target/reference versions for the legacy UWP project. |
| WinIRC.WinUI/WinUI.Desktop.Cs.SingleProjectPackagedApp.vstemplate | Adds a WinUI project template artifact used during porting. |
| WinIRC.WinUI/WinIRC.WinUI.csproj | Defines the packaged WinUI app project, frameworks, packaging, and dependencies. |
| WinIRC.WinUI/Views/SettingsSection.cs | Adds settings section enum for WinUI settings navigation/scrolling. |
| WinIRC.WinUI/Views/SettingsPage.xaml.cs | Implements WinUI settings UI logic backed by the new settings store. |
| WinIRC.WinUI/Views/SettingsPage.xaml | Adds WinUI settings UI layout. |
| WinIRC.WinUI/Views/DetachedSessionWindow.xaml.cs | Adds detached window host for sessions/settings. |
| WinIRC.WinUI/Views/DetachedSessionWindow.xaml | Defines detached window XAML layout. |
| WinIRC.WinUI/Views/ConnectPage.xaml.cs | Implements saved-network CRUD + open session flow for WinUI. |
| WinIRC.WinUI/Views/ConnectPage.xaml | Adds WinUI connect/manage networks UI layout. |
| WinIRC.WinUI/Views/ChannelPage.xaml | Adds WinUI channel transcript UI templates/layouts. |
| WinIRC.WinUI/Utils/NickColorHelper.cs | Implements nick color brush generation for WinUI. |
| WinIRC.WinUI/Styles/ThemeResources.xaml | Adds WinUI theme resource dictionary for shell brushes. |
| WinIRC.WinUI/ShellPage.xaml | Adds WinUI shell layout (menu bar, rails, splitters, TabView, sidebar). |
| WinIRC.WinUI/Services/WindowService.cs | Implements detached-window launching via IWindowService. |
| WinIRC.WinUI/Services/WindowingHelpers.cs | Adds HWND/AppWindow interop helpers for WinUI. |
| WinIRC.WinUI/Services/StatePathResolver.cs | Resolves state directory path for packaged/unpackaged scenarios. |
| WinIRC.WinUI/Services/ShareService.cs | Adds placeholder share service implementation for WinUI. |
| WinIRC.WinUI/Services/ProtocolActivationService.cs | Adds protocol activation state holder for WinUI. |
| WinIRC.WinUI/Services/PackageMigrationService.cs | Implements migration from legacy package state to new JSON stores. |
| WinIRC.WinUI/Services/JsonSettingsStore.cs | Adds JSON settings persistence implementation (ISettingsStore). |
| WinIRC.WinUI/Services/JsonServerRepository.cs | Adds JSON server repository persistence (IServerRepository). |
| WinIRC.WinUI/Services/FolderPickerService.cs | Adds folder picker service with WinUI window initialization. |
| WinIRC.WinUI/Services/ExternalLauncherService.cs | Adds external URI launcher service for WinUI. |
| WinIRC.WinUI/Services/DesktopNotificationService.cs | Adds desktop toast notification service (Windows App SDK notifications). |
| WinIRC.WinUI/Properties/PublishProfiles/win-x86.pubxml | Adds WinUI publish profile for x86. |
| WinIRC.WinUI/Properties/PublishProfiles/win-x64.pubxml | Adds WinUI publish profile for x64. |
| WinIRC.WinUI/Properties/PublishProfiles/win-arm64.pubxml | Adds WinUI publish profile for ARM64. |
| WinIRC.WinUI/Properties/launchSettings.json | Adds debug launch profiles (packaged/unpackaged). |
| WinIRC.WinUI/Package.debug.appxmanifest | Adds side-by-side debug package identity + protocol declarations. |
| WinIRC.WinUI/Package.appxmanifest | Adds release package manifest + protocol declarations. |
| WinIRC.WinUI/Models/SidebarTreeItem.cs | Adds sidebar tree model with unread/mention visuals. |
| WinIRC.WinUI/Models/SessionTab.cs | Adds session tab model with unread state. |
| WinIRC.WinUI/Models/ChannelMessageItem.cs | Adds message view-model used by WinUI channel UI. |
| WinIRC.WinUI/Models/ChannelMessageGroupItem.cs | Adds grouped message model for compact/modern transcript styles. |
| WinIRC.WinUI/Models/ChannelFactItem.cs | Adds label/value model for sidebar facts. |
| WinIRC.WinUI/MainWindow.xaml.cs | Adds main WinUI window startup, title bar theming, debug logging. |
| WinIRC.WinUI/MainWindow.xaml | Adds main WinUI window XAML host. |
| WinIRC.WinUI/Interfaces/ISessionSidebarSource.cs | Defines contract for pages providing sidebar content. |
| WinIRC.WinUI/Assets/Wide310x150Logo.scale-200.png | Adds WinUI package asset. |
| WinIRC.WinUI/Assets/StoreLogo.png | Adds WinUI package asset. |
| WinIRC.WinUI/Assets/Square44x44Logo.targetsize-24_altform-unplated.png | Adds WinUI package asset. |
| WinIRC.WinUI/Assets/Square44x44Logo.scale-200.png | Adds WinUI package asset. |
| WinIRC.WinUI/Assets/Square150x150Logo.scale-200.png | Adds WinUI package asset. |
| WinIRC.WinUI/Assets/LockScreenLogo.scale-200.png | Adds WinUI package asset. |
| WinIRC.WinUI/AppBootstrapContext.cs | Wires up WinUI app services (settings, repo, runtime, migration). |
| WinIRC.WinUI/App.xaml.cs | Adds WinUI App startup + activation handling + crash logging. |
| WinIRC.WinUI/App.xaml | Adds WinUI merged dictionaries (controls + theme resources). |
| WinIRC.WinUI/app.manifest | Adds OS compatibility + DPI awareness for unpackaged scenarios. |
| WinIRC.sln | Adds new projects + configurations to the solution. |
| WinIRC.Core/WinIRC.Core.csproj | Adds shared core library project + IRC client dependency. |
| WinIRC.Core/Services/WorkspaceSessionService.cs | Implements workspace targets + command interpretation. |
| WinIRC.Core/Services/IrcRuntimeService.cs | Implements shared IRC runtime/session state + message handling. |
| WinIRC.Core/Services/IrcRuntimePrimitives.cs | Adds socket/websocket primitives used by the runtime. |
| WinIRC.Core/Models/WorkspaceTarget.cs | Adds model for pinned/transient workspace targets. |
| WinIRC.Core/Models/WorkspaceCommandActionKind.cs | Adds action-kind enum for interpreted commands. |
| WinIRC.Core/Models/WorkspaceCommandAction.cs | Adds command action result model. |
| WinIRC.Core/Models/SettingValue.cs | Adds discriminated-ish setting value model for JSON storage. |
| WinIRC.Core/Models/SettingsSnapshot.cs | Adds settings snapshot model for JSON persistence. |
| WinIRC.Core/Models/SavedServerValidation.cs | Adds saved server normalization/validation logic. |
| WinIRC.Core/Models/SavedServer.cs | Adds saved server model shared by WinUI/runtime. |
| WinIRC.Core/Models/RangeObservableCollection.cs | Adds collection helper for batched UI updates. |
| WinIRC.Core/Models/LegacyWinIrcServer.cs | Adds legacy server model conversion helper. |
| WinIRC.Core/Models/IrcRuntimeTargetInfo.cs | Adds runtime target metadata model. |
| WinIRC.Core/Models/IrcRuntimeSessionState.cs | Adds runtime session state incl. unread/mention tracking. |
| WinIRC.Core/Models/IrcRuntimeMessageKind.cs | Adds runtime message kind enum. |
| WinIRC.Core/Models/IrcRuntimeMessage.cs | Adds runtime message model. |
| WinIRC.Core/Models/ChatSessionKind.cs | Adds session kind enum for WinUI session routing. |
| WinIRC.Core/Models/ChatSessionDescriptor.cs | Adds session descriptor used for tabs/detached windows. |
| WinIRC.Core/Interfaces/IWorkspaceSessionService.cs | Adds workspace session service interface. |
| WinIRC.Core/Interfaces/IWindowService.cs | Adds window service interface. |
| WinIRC.Core/Interfaces/IShareService.cs | Adds share service interface. |
| WinIRC.Core/Interfaces/ISettingsStore.cs | Adds settings store interface for WinUI/runtime. |
| WinIRC.Core/Interfaces/IServerRepository.cs | Adds server repository interface for WinUI/runtime. |
| WinIRC.Core/Interfaces/IProtocolActivationService.cs | Adds protocol activation service interface. |
| WinIRC.Core/Interfaces/INotificationService.cs | Adds notification service interface. |
| WinIRC.Core/Interfaces/IIrcRuntimeService.cs | Adds IRC runtime interface used by WinUI. |
| WinIRC.Core/Interfaces/IFilePickerService.cs | Adds file picker interface. |
| WinIRC.Core/Interfaces/IExternalLauncher.cs | Adds external launcher interface. |
| WinIRC.Core/ConfigKeys.cs | Adds shared config key constants incl. migration keys. |
| README.md | Updates documentation for dual UWP/WinUI repo and build instructions. |
| PORT_STATUS.md | Adds migration/port status tracker. |
| AGENTS.md | Adds repository/architecture guide for future automation/agents. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await _settingsStore.SetBooleanAsync(ConfigKeys.MigrationCompleted, importedData).ConfigureAwait(false); | ||
| await _settingsStore.SetBooleanAsync(ConfigKeys.MigrationImportedData, importedData).ConfigureAwait(false); | ||
| await _settingsStore.SetIntAsync(ConfigKeys.MigrationImportedSettingsCount, importedSettings).ConfigureAwait(false); | ||
| await _settingsStore.SetIntAsync(ConfigKeys.MigrationImportedServersCount, importedServers).ConfigureAwait(false); |
There was a problem hiding this comment.
MigrationCompleted is being set to importedData. If no legacy data is found/imported, this remains false, causing ShouldAttemptMigration to retry migration on every startup (repeatedly scanning package folders). Set MigrationCompleted to true after a migration attempt regardless of whether anything was imported, and keep MigrationImportedData as the flag for whether anything was actually migrated.
| public IReadOnlyDictionary<string, SettingValue> GetAll() | ||
| { | ||
| return _snapshot.Values; | ||
| } |
There was a problem hiding this comment.
GetAll() returns the backing mutable dictionary (_snapshot.Values). Callers can mutate it and bypass persistence/validation, and it also exposes internal state to concurrent modification. Return a defensive copy or a read-only wrapper instead.
| private async Task SaveAsync(CancellationToken cancellationToken) | ||
| { | ||
| Directory.CreateDirectory(Path.GetDirectoryName(_filePath)!); | ||
| await using var stream = File.Create(_filePath); | ||
| await JsonSerializer.SerializeAsync(stream, _snapshot, _serializerOptions, cancellationToken).ConfigureAwait(false); | ||
| } |
There was a problem hiding this comment.
SaveAsync overwrites the same file via File.Create without any synchronization. Because multiple Set*Async calls can run concurrently (and some callers fire-and-forget), overlapping writes can corrupt settings.json or lose updates. Serialize writes with a SemaphoreSlim/lock and consider writing to a temp file then atomically replacing the target.
| _ = _settingsStore.SetStringAsync( | ||
| BuildLastReadKey(channelName), | ||
| timestamp.Value.ToString("O")); | ||
| } |
There was a problem hiding this comment.
This fire-and-forget _settingsStore.SetStringAsync(...) can run concurrently with other settings writes and any exceptions will be unobserved. Given JsonSettingsStore is not thread-safe, this also increases the likelihood of settings file corruption. Consider awaiting this call or queueing/serializing settings writes inside the store.
| private static RuntimeServerDefinition CreateRuntimeServer(SavedServer server) | ||
| { | ||
| return new RuntimeServerDefinition | ||
| { | ||
| Name = server.Name, | ||
| Hostname = server.Hostname, | ||
| Port = server.Port, | ||
| Ssl = server.Ssl, | ||
| IgnoreCertErrors = false, | ||
| ShouldReconnect = false, | ||
| Username = server.Username, | ||
| Password = server.Password, | ||
| NickservPassword = server.NickservPassword, | ||
| Channels = server.Channels, | ||
| UseWebSocket = server.UseWebSocket | ||
| }; |
There was a problem hiding this comment.
CreateRuntimeServer hard-codes IgnoreCertErrors = false and ShouldReconnect = false, even though the WinUI settings UI persists ConfigKeys.IgnoreSSL and ConfigKeys.AutoReconnect. This makes the toggles ineffective and breaks parity with the legacy app. Pull these values from ISettingsStore (or pass them in) when building the runtime server definition and connection behavior.
| public async Task Disconnect(string msg = "Powered by WinIRC", bool attemptReconnect = false) | ||
| { | ||
| if (Interlocked.Exchange(ref _disconnecting, 1) != 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| if (Parent.IsConnected) | ||
| { | ||
| await WriteLine("QUIT :" + msg).ConfigureAwait(false); | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| } | ||
|
|
||
| Parent.IsConnected = false; | ||
| Parent.IsConnecting = false; | ||
|
|
||
| await CloseAsync().ConfigureAwait(false); | ||
| Parent.HandleDisconnect?.Invoke(Parent); | ||
| } |
There was a problem hiding this comment.
The attemptReconnect parameter is accepted by Disconnect(...) but never used, and there is no reconnect scheduling logic in the socket layer. Combined with the UI toggle for auto-reconnect, this is misleading and makes future behavior harder to reason about. Either implement reconnection honoring attemptReconnect/a ShouldReconnect flag, or remove the parameter until reconnect behavior is supported.
| protected override async void OnLaunched(Microsoft.UI.Xaml.LaunchActivatedEventArgs args) | ||
| { | ||
| await EnsureInitializedAsync(GetPendingProtocolUri()); | ||
| } | ||
|
|
||
| private static async Task EnsureInitializedAsync(Uri? pendingProtocolUri = null) | ||
| { | ||
| if (Context is null) | ||
| { | ||
| Context = await AppBootstrapContext.CreateAsync().ConfigureAwait(true); | ||
| } | ||
|
|
||
| if (pendingProtocolUri != null) | ||
| { | ||
| Context.ProtocolActivationService.SetPendingProtocolUri(pendingProtocolUri); | ||
| } | ||
|
|
||
| if (MainWindow is null) | ||
| { | ||
| MainWindow = new MainWindow(); | ||
| } | ||
|
|
||
| MainWindow.Activate(); | ||
| } |
There was a problem hiding this comment.
EnsureInitializedAsync can be entered concurrently from OnLaunched and AppInstance.Activated, which can race creating Context and/or MainWindow. Add synchronization (e.g., a static SemaphoreSlim guarding initialization) to ensure only one initialization path runs at a time.
| ```powershell | ||
| & 'C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe' ` | ||
| 'C:\Users\Jess\source\repos\WinIRC\WinIRC.WinUI\WinIRC.WinUI.csproj' ` | ||
| /restore /t:Build ` | ||
| /p:Configuration=Debug ` | ||
| /p:Platform=x64 ` | ||
| /p:AppxPackage=false ` | ||
| /p:UapAppxPackageBuildMode=SideloadOnly | ||
| ``` |
There was a problem hiding this comment.
This build command hard-codes an absolute local path (C:\Users\Jess\...). Replace it with repository-relative paths (or clearly-marked placeholders) so the README stays usable for other contributors and CI environments.
|
|
||
| WinIRC is an IRC client originally built as a UWP app. This repository now contains both: | ||
|
|
||
| - the legacy UWP application in [`C:\Users\Jess\source\repos\WinIRC\WinIRC`](C:\Users\Jess\source\repos\WinIRC\WinIRC) |
There was a problem hiding this comment.
This doc includes absolute local machine paths (e.g., C:\Users\Jess\source\repos\...) which won't exist for other contributors and can be confusing. Prefer repository-relative links/paths and keep host-specific paths out of tracked documentation.
| - the legacy UWP application in [`C:\Users\Jess\source\repos\WinIRC\WinIRC`](C:\Users\Jess\source\repos\WinIRC\WinIRC) |
| <ProjectTypeTag>desktop</ProjectTypeTag> | ||
| <ProjectTypeTag>WinUI</ProjectTypeTag> | ||
| </TemplateData> | ||
| <TemplateContent PreferedSolutionConfiguration="Debug|x64"> |
There was a problem hiding this comment.
PreferedSolutionConfiguration is misspelled; Visual Studio template schema expects PreferredSolutionConfiguration. With the current spelling, the default configuration may be ignored when creating the project from this template.
Uh oh!
There was an error while loading. Please reload this page.