Add Windows Terminal to Shell Plugins #3225
Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe changes introduce a new property Changes
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (1)
282-308: Implementation looks good but needs additional error handling.The Windows Terminal integration is well-implemented, following proper command-line conventions for both PowerShell and CMD scenarios.
Consider these improvements:
- Verify wt.exe existence before execution
- Add specific error handling for Windows Terminal scenarios
Add this verification before the Windows Terminal cases:
case Shell.TerminalPWSH: { + if (!ExistInPath("wt.exe")) + { + throw new FileNotFoundException("Windows Terminal (wt.exe) is not installed."); + } info.FileName = "wt.exe"; // ... rest of the code } case Shell.TerminalCMD: { + if (!ExistInPath("wt.exe")) + { + throw new FileNotFoundException("Windows Terminal (wt.exe) is not installed."); + } info.FileName = "wt.exe"; // ... rest of the code }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/Settings.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs(2 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- Plugins/Flow.Launcher.Plugin.Shell/Settings.cs
- Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml
- Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs
- Plugins/Flow.Launcher.Plugin.Shell/Main.cs
🔇 Additional comments (7)
Plugins/Flow.Launcher.Plugin.Shell/Settings.cs (3)
42-43: LGTM! Enum values are properly defined.The new enum values for Windows Terminal integration follow consistent naming patterns and are correctly sequenced.
42-43: LGTM! Enum values are well-defined.The new enum values for Windows Terminal integration follow consistent naming conventions and are properly ordered.
42-43: LGTM! Enum values are properly defined.The new shell types for Windows Terminal are correctly added with unique values and follow the existing naming convention.
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs (3)
94-95: LGTM! Selection logic is properly implemented.The selection handling for new Terminal options is correctly implemented and maintains consistency with existing shell options.
Also applies to: 106-107
94-95: LGTM! Selection logic properly handles new terminal options.The switch expressions are correctly updated to handle the new Windows Terminal options while maintaining existing functionality.
Also applies to: 106-107
94-95: LGTM! Selection logic is properly implemented.The selection handling for Windows Terminal options is correctly integrated in both the initial selection and change event handlers.
Also applies to: 106-107
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (1)
282-308: Verify Windows Terminal command-line compatibility.Let's confirm the command-line argument syntax for Windows Terminal.
✅ Verification successful
🌐 Web query:
What are the supported command-line arguments for Windows Terminal?Length of output: 5531
Windows Terminal Command-Line Arguments Are Compatible
- The implementation uses
wt.exewith shell identifiers (pwshfor PowerShell andcmdfor Command Prompt), which is in line with Windows Terminal’s flexible argument parsing.- For PowerShell, using
-Commandwith an optional-NoExitflag (when leaving the shell open) matches PowerShell’s standard argument scheme.- For CMD, the use of
/c(to close after execution) and/k(to keep the shell open) is consistent with CMD’s typical behavior.- According to official documentation and external guides, Windows Terminal supports these patterns when launching specific shells.
| case Shell.TerminalPWSH: | ||
| { | ||
| info.FileName = "wt.exe"; | ||
| info.ArgumentList.Add("pwsh"); | ||
| if (_settings.LeaveShellOpen) | ||
| { | ||
| info.ArgumentList.Add("-NoExit"); | ||
| } | ||
| info.ArgumentList.Add("-Command"); | ||
| info.ArgumentList.Add(command); | ||
| break; | ||
| } | ||
| case Shell.TerminalCMD: | ||
| { | ||
| info.FileName = "wt.exe"; | ||
| info.ArgumentList.Add("cmd"); | ||
| if (_settings.LeaveShellOpen) | ||
| { | ||
| info.ArgumentList.Add("/k"); | ||
| } | ||
| else | ||
| { | ||
| info.ArgumentList.Add("/c"); | ||
| } | ||
| info.ArgumentList.Add(command); | ||
| break; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Windows Terminal availability.
The code assumes wt.exe is available but doesn't handle cases where Windows Terminal isn't installed.
Run this script to check how the code handles missing Windows Terminal:
Consider implementing CloseShellAfterPress behavior.
The new Terminal cases don't implement the CloseShellAfterPress behavior that's available for other shells.
Consider adding the pause behavior similar to other shells:
case Shell.TerminalPWSH:
{
info.FileName = "wt.exe";
info.ArgumentList.Add("pwsh");
if (_settings.LeaveShellOpen)
{
info.ArgumentList.Add("-NoExit");
}
info.ArgumentList.Add("-Command");
- info.ArgumentList.Add(command);
+ info.ArgumentList.Add($"{command}; {(_settings.CloseShellAfterPress ? $"Write-Host '{context.API.GetTranslation("flowlauncher_plugin_cmd_press_any_key_to_close")}'; [System.Console]::ReadKey(); exit" : "")}");
break;
}
case Shell.TerminalCMD:
{
info.FileName = "wt.exe";
info.ArgumentList.Add("cmd");
if (_settings.LeaveShellOpen)
{
info.ArgumentList.Add("/k");
}
else
{
info.ArgumentList.Add("/c");
}
- info.ArgumentList.Add(command);
+ info.ArgumentList.Add($"{command} {(_settings.CloseShellAfterPress ? $"&& echo {context.API.GetTranslation("flowlauncher_plugin_cmd_press_any_key_to_close")} && pause > nul" : "")}");
break;
}🏁 Script executed:
#!/bin/bash
# Search for error handling related to wt.exe
rg -A 5 "wt\.exe"
rg -A 5 "FileNotFoundException"Length of output: 10564
Issues Detected in Windows Terminal Handling
- The code unconditionally uses
wt.exewithout explicitly checking for its presence. Although a genericFileNotFoundExceptionis caught later, there is no proactive verification or more graceful handling if Windows Terminal isn’t installed. - The new cases for TerminalPWSH and TerminalCMD lack implementation for the
CloseShellAfterPressbehavior, which is otherwise available for other shell types.
💡 Verification agent
🧩 Analysis chain
Verify Windows Terminal installation.
Let's check if Windows Terminal is properly installed in the system.
🏁 Script executed:
#!/bin/bash
# Description: Check if Windows Terminal is installed by searching for wt.exe in PATH
# Test: Search for wt.exe in common installation paths
echo "Checking for Windows Terminal installation..."
rg -l "wt\.exe" Length of output: 161
Action Required: Add a Runtime Check for Windows Terminal Availability
The current implementation simply hardcodes the use of "wt.exe" without verifying if Windows Terminal is installed on the system. The shell script output only confirms that the string "wt.exe" appears in the code (in Plugins/Flow.Launcher.Plugin.Shell/Main.cs), but it does not verify that Windows Terminal is actually available at runtime.
• The code in Plugins/Flow.Launcher.Plugin.Shell/Main.cs uses "wt.exe" to launch Windows Terminal without checking for its existence, which could lead to failures on systems where Windows Terminal is not installed.
Please add a runtime check (for example, using Windows system API calls or checking the system PATH) to verify the availability of "wt.exe" before attempting to launch it.
💡 Verification agent
🧩 Analysis chain
LGTM! Windows Terminal integration is well implemented.
The implementation correctly handles both PowerShell and CMD execution through Windows Terminal.
However, we should verify that Windows Terminal is available before attempting to use it. Run this script to check for Windows Terminal availability:
Consider adding a check for Windows Terminal availability and appropriate error handling:
case Shell.TerminalPWSH:
{
+ if (!ExistInPath("wt.exe"))
+ {
+ throw new FileNotFoundException("Windows Terminal (wt.exe) is not installed");
+ }
info.FileName = "wt.exe";
info.ArgumentList.Add("pwsh");🏁 Script executed:
#!/bin/bash
# Description: Check if Windows Terminal is available in the system
# Search for wt.exe in the PATH
rg -l "where.*wt\.exe" || echo "No existing checks for wt.exe found"
# Search for any Windows Terminal related error handling
rg -l "WindowsTerminal|wt\.exe.*not.*found" || echo "No existing error handling for Windows Terminal found"Length of output: 268
Windows Terminal Availability Check is Missing
The integration works as intended for both PowerShell and CMD, but our check confirms there’s no verification for the existence of wt.exe. This could lead to runtime failures if Windows Terminal is not installed.
- Location: Plugins/Flow.Launcher.Plugin.Shell/Main.cs (lines 282–308)
- Issue: No error handling or pre-execution check to verify that wt.exe exists in the system’s PATH.
- Recommendation: Add a check (e.g., using a helper function like
ExistInPath("wt.exe")) and throw a descriptive exception if wt.exe is not available.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Plugins/Flow.Launcher.Plugin.Shell/Languages/en.xaml(2 hunks)Plugins/Flow.Launcher.Plugin.Shell/Languages/es-419.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/Languages/es.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/Main.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Shell/Settings.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml(2 hunks)Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Plugins/Flow.Launcher.Plugin.Shell/Settings.cs
- Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs
- Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml
- Plugins/Flow.Launcher.Plugin.Shell/Main.cs
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.Shell/Languages/es-419.xaml (1)
9-9: LGTM!The translation is accurate and the resource key is consistent with other Spanish language files.
Plugins/Flow.Launcher.Plugin.Shell/Languages/es.xaml (1)
9-9: LGTM!The translation is accurate and the resource key is consistent with other Spanish language files.
|
I was left unsatisfied by my work of yesterday, so I switched things around a little bit, using a checkbox for using the windows terminal instead of separate entries in the combobox I'm not quite sure how the different language localization works, but I did add English and Spanish strings, which are the languages I know |
There was a problem hiding this comment.
Thanks for those translations. However, language files other than English need to be added by Crowdin. So you have to revert all changes in those files...
Jack251970
added
enhancement
Hi!
I left a comment couple of comments on #2045, but I decided to add the changes myself in my own pull request since the original one was rather old
I've never worked in a C# or windows project before, so please excuse if I messed something up in the process
I've built it and it seems to just work, which is a win in my books.
Hope it helps!
It is a much simpler fix compared to #1116, but it gets the job done