cmderdev / cmder

Lovely console emulator package for Windows
https://cmder.app
MIT License
25.74k stars 2.02k forks source link

Add support for inheriting Clink Profile from globally installed Clink #2858

Open daxgames opened 1 year ago

daxgames commented 1 year ago

Suggestion

Cmder should somehow recognize if Clink is already injected and inherit the profile path instead of using %CMDER_CONFIG_DIR%.

Use case

If Clink is injected globally on cmd, the history should available if using cmder while maintaining the the Cmder [rompt.

Extra info/examples/attachments

No response

Checklist

daxgames commented 1 year ago

@chrisant996 @DRSDavidSoft @pmsobrado Hopefully I captured the requirements. Let the solutioning begin!

chrisant996 commented 1 year ago

Some thoughts:

  1. I recommend for it to be configurable which Clink profile directory Cmder uses.
  2. I recommend to not change the default behavior of Cmder -- changing it would create a compatibility/migration problem for existing users of Cmder who may have put scripts and/or custom settings in their Cmder profile directory already.
  3. If Cmder will allow using a non-Cmder profile directory, then the init.bat script will need to avoid modifying non-Cmder profile directories.
    • No cleaning up "old" files. Old and new Clink's can coexist using the same profile, and cleaning up "old" files breaks old Clinks that are using the same profile.
    • No forcibly creating an ..\opt\ directory or ..\bin\ directory above the non-Cmder profile directory.
daxgames commented 1 year ago

Thinking about it, I wonder if this is truly necessary since the new, yet to be merged, #2825 branch more_speed_2 gives the user total control over Clink injection. The user can edit the %CMDER_CONFIG_DIR%\user_init.cmd to point to whatever Clink profile they want.

daxgames commented 1 year ago

If we did decide to do this we would still need to process the %CMDER_ROOT%\vendor\clink.lua and %CMDER_CONFIG_DIR%\cmder_prompt_config.lua.

chrisant996 commented 1 year ago

If we did decide to do this we would still need to process the %CMDER_ROOT%\vendor\clink.lua and %CMDER_CONFIG_DIR%\cmder_prompt_config.lua.

Yes, i.e. Cmder would still need to use --scripts.

chrisant996 commented 1 year ago

Uh oh ... the code in Cmder's custom clink.lua doesn't work correctly for loading scripts from a user-specified profile directory in %CMDER_USER_CONFIG%. It has a hard-coded assumption about where the config directory is, and it loads scripts from that hard-coded place.

So that would need to be changed as well.

chrisant996 commented 1 year ago

Ah, it's a regression introduced in #1884 when fixing #1882. See here for details, and the code (and diffs) to fix the regression.

DRSDavidSoft commented 1 year ago

If I may interject, @chrisant996 why not make a PR since you already took the effort to write a fix and a patch 😅 Otherwise, @daxgames could you please apply it? Thank you to you both

chrisant996 commented 1 year ago

If I may interject, @chrisant996 why not make a PR since you already took the effort to write a fix and a patch 😅 Otherwise, @daxgames could you please apply it? Thank you to you both

I can see where the question is coming from.

But, no, because:

  1. I haven't verified the code; that needs to be done by someone still.
  2. Making a PR (not to mention verifying stuff) takes further additional time that I'm not willing to spend.

I don't use Cmder. I constrain the time I spend on contributions to it. Obviously I spent a lot on this issue, but I've reached the end of what I'm willing to spend on it for a while.

In a few weeks if the issue hasn't been addressed yet, maybe I'll find time to work on it further.

DRSDavidSoft commented 1 year ago

Thanks for taking the time to investigate the issues and writing fixes. 🎉

daxgames commented 1 year ago

I'll take what @chrisant996 did and verify and open a pr.