Skip to content

Fix passing special characters to console-mode launcher on Windows#180

Merged
lassoan merged 1 commit intocommontk:mainfrom
lassoan:fix-special-characters-in-command-line
Feb 19, 2026
Merged

Fix passing special characters to console-mode launcher on Windows#180
lassoan merged 1 commit intocommontk:mainfrom
lassoan:fix-special-characters-in-command-line

Conversation

@lassoan
Copy link
Member

@lassoan lassoan commented Feb 19, 2026

Special characters in command-line arguments on Windows were only converted correctly in the non-console launcher. Now on Windows, command-line arguments are always converted to classic unix-style argc/argv with UTF-8 encoding.

@lassoan lassoan requested a review from cpinter February 19, 2026 15:14
@lassoan lassoan self-assigned this Feb 19, 2026
@lassoan lassoan force-pushed the fix-special-characters-in-command-line branch from 840b25a to a5ba595 Compare February 19, 2026 15:27
@lassoan
Copy link
Member Author

lassoan commented Feb 19, 2026

@jcfr how can we create a new AppLauncher release after we merge this?

@jamesobutler
Copy link
Contributor

jamesobutler commented Feb 19, 2026

The CD workflow gets triggered on the creation of a GitHub release as of 91f3d8e. Or manually triggered by specifying a tag and running the workflow through workflow dispatch.
{889001B7-5B98-4BA3-B158-589501538A53}

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes special character handling in Windows console-mode launcher by ensuring command-line arguments are consistently converted to UTF-8 encoding on Windows across all launcher modes (console and GUI).

Changes:

  • Added wmain entry point for Windows console launcher to properly handle wide-character arguments
  • Refactored argument conversion logic to use shared helper functions
  • Introduced platform-specific encoding/decoding helpers in ctkAppArguments for consistent UTF-8 handling on Windows

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
Main.cpp Added wmain entry point for Windows console mode with proper UTF-8 argument conversion; refactored cleanup to use deleteUnixArguments
Base/ctkCommandLineParser.h Added overload for convertWindowsCommandLineToUnixArguments and deleteUnixArguments function declarations
Base/ctkCommandLineParser.cpp Implemented helper functions for UTF-8 conversion, refactored existing conversion logic to reuse helpers, added cleanup function
Base/ctkAppArguments.cpp Added platform-specific decodeArgument/encodeArgument helpers to ensure consistent UTF-8 handling on Windows

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jcfr
Copy link
Member

jcfr commented Feb 19, 2026

To follow-up on @jamesobutler comment, the release pipeline should be fully automated :)

@lassoan lassoan force-pushed the fix-special-characters-in-command-line branch from a5ba595 to bf7c6d5 Compare February 19, 2026 16:37
@lassoan
Copy link
Member Author

lassoan commented Feb 19, 2026

To follow-up on @jamesobutler comment, the release pipeline should be fully automated :)

Awesome! Thanks for the responses @jamesobutler and @jcfr.

I've found the build binaries in the CI artifacts and tested that everything works well on Windows now!
Once CI is finished successfully I'll merge.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lassoan lassoan force-pushed the fix-special-characters-in-command-line branch from bf7c6d5 to 256c643 Compare February 19, 2026 16:54
@lassoan lassoan requested a review from Copilot February 19, 2026 16:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

Base/ctkCommandLineParser.cpp:978

  • In convertWindowsCommandLineToUnixArguments(), the early guard returns before initializing *argc/*argv when cmd_line is null. Callers (e.g., wWinMain) pass uninitialized locals and appLauncherMain later reads argv[0], so this can lead to undefined behavior/crash. Consider always setting *argc = 0 and *argv = nullptr first, and if cmd_line is null fall back to returning at least argv[0] (empty or module filename).
void ctkCommandLineParser::convertWindowsCommandLineToUnixArguments(PWSTR cmd_line, int* argc, char*** argv)
{
  if (!cmd_line || !argc || !argv)
  {
    return;
  }
  *argc = 0;
  *argv = nullptr;


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lassoan lassoan force-pushed the fix-special-characters-in-command-line branch from 256c643 to ea1fae8 Compare February 19, 2026 17:33
@lassoan lassoan requested a review from Copilot February 19, 2026 17:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* to an empty string.
*
* @note The first argument (the executable path) will be set from the executable
* path determined by ::GetModuleFileNameW. The original value of argv[0] is not used.
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation incorrectly refers to "argv[0]" when it should refer to "wideArgv[0]". The parameter "argv" is an output parameter (where the converted arguments are stored), while "wideArgv" is the input parameter. The comment should read: "The original value of wideArgv[0] is not used."

Suggested change
* path determined by ::GetModuleFileNameW. The original value of argv[0] is not used.
* path determined by ::GetModuleFileNameW. The original value of wideArgv[0] is not used.

Copilot uses AI. Check for mistakes.
Special characters in command-line arguments on Windows were only converted correctly in the non-console launcher.
Now on Windows, command-line arguments are always converted to classic unix-style argc/argv with UTF-8 encoding.
@lassoan lassoan force-pushed the fix-special-characters-in-command-line branch from ea1fae8 to b7d59d7 Compare February 19, 2026 17:41
@lassoan lassoan requested a review from Copilot February 19, 2026 17:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lassoan lassoan merged commit 61c35e7 into commontk:main Feb 19, 2026
13 checks passed
@lassoan lassoan deleted the fix-special-characters-in-command-line branch February 19, 2026 17:59
@jamesobutler
Copy link
Contributor

jamesobutler commented Feb 19, 2026

@lassoan Full release steps are documented at the following below including updating the CMakeLists.txt with the bumped version corresponding to the new tag that is then created.

https://github.com/commontk/AppLauncher/blob/61c35e7516a87a96032cba6e6672a0728f7c55fa/README.md#maintainers-how-to-make-a-release-

@lassoan
Copy link
Member Author

lassoan commented Feb 19, 2026

I assumed that's the old manual process :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants