cmder icon indicating copy to clipboard operation
cmder copied to clipboard

Add support for inheriting Clink Profile from globally installed Clink

Open daxgames opened this issue 2 years ago • 11 comments

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

  • [X] I have read the documentation and made sure this feature doesn't already exist.
  • [X] I have searched for similar issues and found none that describe my feature request.

daxgames avatar Jul 04 '23 18:07 daxgames

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

daxgames avatar Jul 04 '23 18:07 daxgames

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.

chrisant996 avatar Jul 04 '23 18:07 chrisant996

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 avatar Jul 04 '23 18:07 daxgames

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.

daxgames avatar Jul 04 '23 18:07 daxgames

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 avatar Jul 04 '23 18:07 chrisant996

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 avatar Jul 04 '23 19:07 chrisant996

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

chrisant996 avatar Jul 04 '23 19:07 chrisant996

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

DRSDavidSoft avatar Jul 04 '23 19:07 DRSDavidSoft

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.

chrisant996 avatar Jul 04 '23 20:07 chrisant996

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

DRSDavidSoft avatar Jul 04 '23 20:07 DRSDavidSoft

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

daxgames avatar Jul 04 '23 20:07 daxgames