Cmder development changes #2898
Draft
+1,554
−264
Conversation
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
Sync with master
There was a problem hiding this comment.
Pull Request Overview
Add support for Windows Terminal alongside ConEmu and enable packing/building specific terminals via a new -terminal option. Updates include:
- Introduce
-terminalparameter inpack.ps1andbuild.ps1to select which emulator artifacts to include. - Extend the C++ launcher (
CmderLauncher.cpp) to detect and launch Windows Terminal, migrate config logic, and renameconEmureferences to genericterminal. - Update CI workflow to produce and upload separate artifacts for each terminal type and adjust documentation and packaging rules.
Reviewed Changes
Copilot reviewed 11 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/pack.ps1 | Added $terminal param and conditional target sets; duplicate branch. |
| scripts/build.ps1 | Added $terminal param, Windows Terminal backup/restore logic; used return inside loop. |
| packignore | Include new config\user_* pattern. |
| launcher/src/strings.rc2 | Updated switch help to include Windows Terminal flags. |
| launcher/src/resource.rc | Adjusted #endif comment label. |
| launcher/src/CmderLauncher.cpp | Renamed variables and added Windows Terminal detection/logic; scattered typos and grouping bug. |
| launcher/CmderLauncher.vcxproj | Added icon image entry to project. |
| README.md | Updated usage docs to use -- instead of /X. |
| Cmder.bat | Dispatch to ConEmu or Windows Terminal based on existence. |
| CHANGELOG.md | Added unreleased section documenting the new Windows Terminal support. |
| .github/workflows/build.yml | Updated build steps to pass -terminal and upload new artifacts. |
Comment on lines
+59
to
+63
| } elseif ($terminal -eq "windows-terminal") { | ||
| $targets = @{ | ||
| "cmder.7z" = "-t7z -m0=lzma2 -mx=9 -mfb=64 -md=32m -ms=on -myx=7 -mqs=on -xr!`"vendor\windows-terminal`""; | ||
| "cmder.zip" = "-mm=Deflate -mfb=128 -mpass=3 -xr!`"vendor\windows-terminal`""; | ||
| "cmder_mini.zip" = "-xr!`"vendor\git-for-windows`" -xr!`"vendor\windows-terminal`""; |
There was a problem hiding this comment.
This branch duplicates the previous elseif for windows-terminal. It looks like one should target conemu-maximus5 instead of repeating windows-terminal.
Suggested change
| } elseif ($terminal -eq "windows-terminal") { | |
| $targets = @{ | |
| "cmder.7z" = "-t7z -m0=lzma2 -mx=9 -mfb=64 -md=32m -ms=on -myx=7 -mqs=on -xr!`"vendor\windows-terminal`""; | |
| "cmder.zip" = "-mm=Deflate -mfb=128 -mpass=3 -xr!`"vendor\windows-terminal`""; | |
| "cmder_mini.zip" = "-xr!`"vendor\git-for-windows`" -xr!`"vendor\windows-terminal`""; | |
| } elseif ($terminal -eq "conemu-maximus5") { | |
| $targets = @{ | |
| "cmder_conemu.7z" = "-t7z -m0=lzma2 -mx=9 -mfb=64 -md=32m -ms=on -myx=7 -mqs=on -xr!`"vendor\windows-terminal`""; | |
| "cmder_conemu.zip" = "-mm=Deflate -mfb=128 -mpass=3 -xr!`"vendor\windows-terminal`""; | |
| "cmder_conemu_mini.zip" = "-xr!`"vendor\git-for-windows`" -xr!`"vendor\windows-terminal`""; |
| } | ||
| } | ||
| else if (_wcsicmp(L"/task", szArgList[i]) == 0) | ||
| else if (_wcsicmp(L"/task", szArgList[i]) == 0 || PathFileExists(windowsTerminalDir) || PathFileExists(conEmuDir)) |
There was a problem hiding this comment.
The OR conditions will match whenever a terminal exists, not just on the /task flag. You likely want to group the checks as (_wcsicmp(...) == 0) && (PathFileExists(...) || ...) to avoid unintended matches.
Suggested change
| else if (_wcsicmp(L"/task", szArgList[i]) == 0 || PathFileExists(windowsTerminalDir) || PathFileExists(conEmuDir)) | |
| else if (_wcsicmp(L"/task", szArgList[i]) == 0 && (PathFileExists(windowsTerminalDir) || PathFileExists(conEmuDir))) |
|
@copilot Hmm, I appreciate the detailed review, I'll address the issues raised in a local branch. |
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.
No description provided.