Launch multi-command workspaces as tabs instead of separate windows#13
Open
tonythethompson wants to merge 4 commits into
Open
Launch multi-command workspaces as tabs instead of separate windows#13tonythethompson wants to merge 4 commits into
tonythethompson wants to merge 4 commits into
Conversation
Workspaces with multiple enabled launch entries used to open one Windows Terminal window per entry. Now entries that resolve to the same Windows Terminal host and elevation requirement are combined into a single wt.exe/wtai.exe invocation using `new-tab`, so a workspace with several commands opens as tabs in one window instead of separate windows. Entries needing different elevation, or that fall back to a non-tab-capable host (standalone PowerShell/cmd/wsl, only used when Windows Terminal isn't installed), still launch as separate windows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QA3QpUk1crPby6PYjnZJ2Y
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates QuickShell’s workspace launcher so multi-entry workspaces open multiple commands as tabs within a single Windows Terminal window (when possible), instead of spawning one window per entry. This improves UX by reducing window spam while preserving the existing behavior for mixed hosts and mixed elevation.
Changes:
- Refactors
TerminalLauncherto separate validation/target resolution (Resolve) from launching (OpenResolved/OpenGroup) and reuses per-entry Windows Terminal argument building. - Updates
ShortcutLaunchExecutor.LaunchAllto resolve entries up front, group tab-capable entries by(HostExecutable, EffectiveElevation), and launch each group as a single process with; new-tab. - Adds unit tests verifying single-process tab launches, elevation splitting, and mixed-host splitting.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| QuickShell.Core/Services/TerminalLauncher.cs | Adds Resolve, OpenResolved, and OpenGroup to support launching multiple workspace entries as Windows Terminal tabs. |
| QuickShell.Core/Services/ShortcutLaunchExecutor.cs | Groups resolved launch plans by host/elevation and launches groups as single processes (tabs) or separate windows (fallback). |
| QuickShell.Core.Tests/ShortcutLaunchExecutorTests.cs | Adds tests asserting tab grouping behavior, elevation grouping behavior, and mixed-host behavior. |
Comment on lines
+79
to
+83
| public static void OpenGroup(IReadOnlyList<ResolvedLaunch> group, bool effectiveElevation) | ||
| { | ||
| var hostExecutable = group[0].Target.HostExecutable; | ||
| var allArguments = new List<string>(); | ||
|
|
Comment on lines
10
to
+12
| internal static Func<ProcessStartInfo, bool>? StartProcessOverride { get; set; } | ||
|
|
||
| public static void Open( | ||
| public static ResolvedLaunch Resolve( |
OpenGroup now fails fast with a clear ArgumentException if called with an empty group or entries that aren't all Windows Terminal-hosted with a matching host executable, instead of silently misbehaving. Group the two test classes that mutate the shared static TerminalLauncher.StartProcessOverride hook into one xUnit collection so they can't run concurrently and race on that shared state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QA3QpUk1crPby6PYjnZJ2Y
Comment on lines
+11
to
+12
| [CollectionDefinition("TerminalLauncher StartProcessOverride", DisableParallelization = true)] | ||
| public sealed class TerminalLauncherOverrideCollection; |
| throw new ArgumentException("Group must contain at least one resolved launch.", nameof(group)); | ||
| } | ||
|
|
||
| var hostExecutable = group[0].Target.HostExecutable; |
Comment on lines
+92
to
+94
| if (target.Kind is not (LaunchTargetKind.WindowsTerminal or LaunchTargetKind.IntelligentTerminal) | ||
| || !target.HostExecutable.Equals(hostExecutable, StringComparison.OrdinalIgnoreCase)) | ||
| { |
| continue; | ||
| } | ||
|
|
||
| var key = (plan.Resolved.Target.HostExecutable, plan.EffectiveElevation); |
- Fix CollectionDefinition placeholder class: a bodyless semicolon declaration isn't valid for a plain (non-record) class. - OpenGroup now fails fast if a resolved target has no host executable, and uses a null-safe string.Equals for the host-executable comparison instead of an instance .Equals call. - Normalize the HostExecutable grouping key (case-insensitive, null-safe) so otherwise-compatible Windows Terminal entries aren't split into separate groups by casing differences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QA3QpUk1crPby6PYjnZJ2Y
Gives a way to manually fire CI from the Actions tab or API when a push doesn't generate a synchronize webhook (observed with non-standard push paths where the ref updates on GitHub but no event is delivered). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QA3QpUk1crPby6PYjnZJ2Y
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.
Summary
(HostExecutable, EffectiveElevation): only entries resolving towt.exe/wtai.exe(Windows Terminal / Intelligent Terminal) can share tabs, since elevation (Run as Admin) applies to the whole window. A singlewt.exe/wtai.exeprocess is launched per group using the documented; new-tabCLI syntax, one tab per entry.ShortcutLaunchExecutor's success/failure messaging now counts launched commands (e.g. "Workspace partially launched: 2 of 3 commands launched") instead of "terminals opened", since a single terminal window can now host multiple commands.Implementation
TerminalLauncher.cs: split validation +LaunchTargetresolution out ofOpeninto a newResolve(...)(returns aResolvedLaunch). AddedOpenResolved(single window, used for singleton groups) andOpenGroup(combines multiple resolved Windows Terminal entries into one process/window with tabs). The Windows Terminal argument builders were refactored to returnList<string>so both paths share the same per-entry argument construction.Open's public signature is unchanged, so existing single-entry callers (LaunchEntry,OpenShortcutLaunchCommand) are unaffected.ShortcutLaunchExecutor.cs:LaunchAllnow resolves each enabled entry, groups tab-capable entries by host executable + elevation (preserving order), and launches each group as one process.Test plan
ShortcutLaunchExecutorTests.csusing the existingTerminalLauncher.StartProcessOverridetest hook:; new-tabseparators.dotnet test QuickShell.Core.Tests— could not be run in this sandbox (no .NET SDK available); please run in CI/locally on Windows before merging.Generated by Claude Code