Refactor Docker transport into shared base classes and strategy pattern#1579
Open
WardenGnaw wants to merge 3 commits into
Open
Refactor Docker transport into shared base classes and strategy pattern#1579WardenGnaw wants to merge 3 commits into
WardenGnaw wants to merge 3 commits into
Conversation
Extract shared container transport infrastructure to enable additional container runtimes without duplicating Docker code. Key changes: - `ContainerTransportSettingsBase` shared abstract base eliminates duplication in transport settings (exe name, host flag, command format) - `IContainerDiscoveryStrategy` interface with Docker implementation keeps runtime-specific logic out of the ViewModel - `ContainerRuntimeType` enum threaded through port picker -> ConnectionManager -> dialog for future extensibility - XAML bindings changed from static resources to ViewModel properties so labels can vary per runtime No behavioral changes - Docker works exactly as before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the SSHDebugPS Docker container picker and transport settings to enable future support for additional container runtimes (e.g., Podman) by centralizing transport-command composition and isolating discovery/runtime-specific behavior behind a strategy interface.
Changes:
- Introduces
ContainerTransportSettingsBase(+ derived base types) and updates Docker transport settings to inherit from the shared base classes. - Adds
ContainerRuntimeTypeplumbing through the port picker andConnectionManagerinto the container picker dialog/ViewModel. - Adds
IContainerDiscoveryStrategywith a Docker implementation and updates XAML to bind label/tooltip text via the ViewModel.
Show a summary per file
| File | Description |
|---|---|
| src/SSHDebugPS/UI/ViewModels/ContainerPickerViewModel.cs | Adds runtime type + discovery strategy selection; exposes runtime-specific label/tooltip text for XAML bindings. |
| src/SSHDebugPS/UI/ContainerPickerDialogWindow.xaml.cs | Threads ContainerRuntimeType into the dialog so the ViewModel can choose the right strategy. |
| src/SSHDebugPS/UI/ContainerPickerDialogWindow.xaml | Switches label/tooltip/automation-name text from static resources to ViewModel-bound properties. |
| src/SSHDebugPS/IContainerDiscoveryStrategy.cs | Introduces the discovery strategy interface used by the picker ViewModel. |
| src/SSHDebugPS/Docker/DockerDiscoveryStrategy.cs | Implements Docker container discovery + platform assignment behind the strategy interface. |
| src/SSHDebugPS/ContainerRuntimeType.cs | Adds an enum to identify which container runtime to query. |
| src/SSHDebugPS/ConnectionManager.cs | Updates container picker window launch API to accept a runtime type. |
| src/SSHDebugPS/Docker/DockerPortPicker.cs | Passes runtime type into ConnectionManager.ShowContainerPickerWindow. |
| src/SSHDebugPS/ContainerTransportSettingsBase.cs | Adds shared base classes for container transport/exec/copy/command settings. |
| src/SSHDebugPS/Docker/TransportSettings/DockerTransportSettings.cs | Refactors Docker command settings to use shared transport settings base. |
| src/SSHDebugPS/Docker/TransportSettings/DockerContainerTransportSettings.cs | Refactors Docker container/exec/copy settings to shared base classes; defines docker exe/host flag constants. |
| src/SSHDebugPS/Docker/DockerHelper.cs | Generalizes the local command runner helper to accept IPipeTransportSettings. |
| src/SSHDebugPS/Docker/DockerExecutionManager.cs | Generalizes base settings type and adds a virtual factory method for exec settings creation. |
| src/SSHDebugPS/Docker/DockerConnection.cs | Broadens runner factory to accept IPipeTransportSettings instead of Docker-only settings. |
| src/SSHDebugPS/Docker/DockerContainerInstance.cs | Makes construction/properties extensible for derived container instance types. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 15/15 changed files
- Comments generated: 2
- Remove unused _runtimeType field from ContainerPickerViewModel - Add explicit type check in DockerExecutionManager.CreateExecSettings instead of unsafe cast Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WardenGnaw
force-pushed
the
dev/waan/podman
branch
from
b1624de to
dfc3bc9
Compare
- Seal DockerDiscoveryStrategy, DockerCommandSettings, DockerContainerTransportSettings, DockerExecSettings, DockerCopySettings - Use StringComparison.OrdinalIgnoreCase for Windows check in AssignPlatforms instead of ToTitleCase + Contains - Remove default parameter values for ContainerRuntimeType to force explicit runtime selection by callers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| string containerPlatform = string.Empty; | ||
| if (DockerHelper.TryGetContainerPlatform(hostname, container.Name, out containerPlatform)) | ||
| { | ||
| container.Platform = new CultureInfo("en-US", false).TextInfo.ToTitleCase(containerPlatform); |
Member
gregg-miskelly
approved these changes
Jun 11, 2026
gregg-miskelly
left a comment
gregg-miskelly
left a comment
Member
There was a problem hiding this comment.
Otherwise looks good to me
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactoring the DockerContainer code to be extensible for other Container tools such as Podman.
Key changes:
ContainerTransportSettingsBaseshared abstract base eliminates duplication in transport settings (exe name, host flag, command format)IContainerDiscoveryStrategyinterface with Docker implementation keeps runtime-specific logic out of the ViewModelContainerRuntimeTypeenum threaded through port picker -> ConnectionManager -> dialog for future extensibility