Skip to content

revert unnecessary changes.#3096

Open
daxgames wants to merge 5 commits into
cmderdev:developmentfrom
daxgames:revert-template-changes
Open

revert unnecessary changes.#3096
daxgames wants to merge 5 commits into
cmderdev:developmentfrom
daxgames:revert-template-changes

Conversation

@daxgames

Copy link
Copy Markdown
Member

No description provided.

@DRSDavidSoft

Copy link
Copy Markdown
Contributor

@daxgames Thank you for the clean PR to fix my mistake 🙏🏻

The %print_debug% removals are especially a large chunk of the bloat, I'm just skimming through any other additions I've caused then merge this before continuing on development


:: Check if Clink is not present
if not exist "%CMDER_ROOT%\vendor\clink\clink_%clink_architecture%.exe" (
%print_error% "Clink executable is not present in 'vendor\clink\clink_%clink_architecture%.exe'"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@daxgames Assuming the Cmder directory content hasn't changed, correct?

Also, I'm considering whether the if branch in batch causes slow downs, if it does, I need to pay more attention to usages like this in other places as well

Comment on lines -44 to -53
:: Cleanup legacy Clink Settings file
if exist "%CMDER_CONFIG_DIR%\settings" if exist "%CMDER_CONFIG_DIR%\clink_settings" (
del "%CMDER_CONFIG_DIR%\settings"
)

:: Cleanup legacy Clink history file
if exist "%CMDER_CONFIG_DIR%\.history" if exist "%CMDER_CONFIG_DIR%\clink_history" (
del "%CMDER_CONFIG_DIR%\.history"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@daxgames These legacy clean ups are one time jobs that meant to be executed from init.bat once instead of every time by init_user.cmd, correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes

Comment on lines +69 to +70
:: Add Windows Terminal shell integration support (OSC 133 sequences)
if defined WT_SESSION (prompt `$e]133;D`$e\`$e]133;A`$e\`$e]9;9;`$P`$e\%PROMPT%`$e]133;B`$e\)

@DRSDavidSoft DRSDavidSoft Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@daxgames I think these OSC definitions should be moved before the :CLINK_FINISH and still be kept under :SKIP_CLINK, because Clink shells don't make use of native Windows prompt anyway, so they might add to the unnecessary command execution

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok - you know more about it than I do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rule of thumb, clink never uses the native Windows prompt.

In fact the prompt <...> command was removed in Cmder circa introduction of Clink, which is excellent. However on platforms where clink wasn't working (e.g. ARM) the native prompt was added as a fallback to avoid making the shell look vanilla instead of our own custom prompt.

It is still useful in my opinion when Clink isn't available, or disabled due to users' preferences.

@DRSDavidSoft

Copy link
Copy Markdown
Contributor

Will come back to this PR later, have to take care of some tasks 🙏🏻
Appreciate the help with aligning the init scripts; I should have worked on these much sooner

@daxgames

Copy link
Copy Markdown
Member Author

still cleaning it up - do not merge yet

) else if exist "%GIT_INSTALL_ROOT%\mingw64" (
set "git_mingw_bin=%GIT_INSTALL_ROOT%\mingw64\bin"
)
:: TODO: Support for ARM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Related ARM for Clink are in #3018, I'm looking forward to getting that merged into development as well, especially with the relevance of more and more ARM machines, like Microsoft's own Surface line

Keeping these here for my own reference

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants