Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
daxgames opened this issue Jul 4, 2023 · 11 comments
Open
2 tasks done

Comments

@daxgames
Copy link
Member

daxgames commented Jul 4, 2023

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

  • I have read the documentation and made sure this feature doesn't already exist.
  • I have searched for similar issues and found none that describe my feature request.
@daxgames
Copy link
Member Author

daxgames commented Jul 4, 2023

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

@chrisant996
Copy link
Contributor

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
Copy link
Member Author

daxgames commented Jul 4, 2023

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
Copy link
Member Author

daxgames commented Jul 4, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

DRSDavidSoft commented Jul 4, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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

@daxgames
Copy link
Member Author

daxgames commented Jul 4, 2023

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

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

No branches or pull requests

3 participants