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

Cmder build 1114 and clink autorun install - error message #2536

Closed
1 of 3 tasks
andyc56-mso opened this issue May 2, 2021 · 5 comments
Closed
1 of 3 tasks

Cmder build 1114 and clink autorun install - error message #2536

andyc56-mso opened this issue May 2, 2021 · 5 comments

Comments

@andyc56-mso
Copy link

andyc56-mso commented May 2, 2021

Purpose of the issue

  • Bug report (encountered problems/errors)
  • Feature request (request for new functionality)
  • Question

Version Information

Cmder version: cmder_mini.zip build 1.0.1114-master from Cmder AppVeyor page (latest build as of now)

Clink version: 1.2.4 (Happens with 1.23 and 1.22 as well, but I didn't go back further than that).

ConEmu version: 210304 [64] (Preview)

Windows version: Windows 10 Pro version 20H2, build 19042.964

Description of the issue

Steps to repeat:

  1. Open up a PowerShell tab in cmder as Administrator
  2. Navigate to cmder\vendor\clink directory
  3. Perform the command: .\clink.bat autorun install
  4. Open up a new (cmd::Cmder) prompt window
  5. The following error message appears:

C:\programs-no-uac\cmder\vendor\clink.lua:90: attempt to concatenate global 'lamb_color' (a nil value)
stack traceback:
C:\programs-no-uac\cmder\vendor\clink.lua:90: in function '?'
?: in function 'filter'
?: in function <?:22>
[C]: in function 'xpcall'
?: in function <?:12>

Steps to get rid of the error message:

  1. Close the (cmd::Cmder) window
  2. In the PowerShell Administrator command window, run the command: .\clink.bat autorun uninstall
  3. Open up a new (cmd::Cmder) prompt window.
  4. No more error message

I wasn't sure whether to report this as a Cmder or Clink issue, but since the error referred to the cmder\vendor\clink.lua file and not something in the cmder\vendor\clink directory, I decided to post it here.

@chrisant996
Copy link
Contributor

chrisant996 commented May 2, 2021

I wasn't sure whether to report this as a Cmder or Clink issue, but since the error referred to the cmder\vendor\clink.lua file and not something in the cmder\vendor\clink directory, I decided to post it here.

An astute observation, thank you! Yes, the cmder\vendor\clink.lua script is part of Cmder, not part of Clink.

It turns out, though, that this reveals two issues: one in Clink itself + one in Cmder.

Issue in Clink:

  • Prior to Clink v1.1.32, Cmder didn't work with Clink set to autorun. Or rather, Cmder ended up hosting a plain cmd+Clink without any of Cmder's extensions (config, scripts, prompt, etc).
  • In Clink v1.1.32, I made it so when Cmder tries to inject Clink into a cmd that already injected Clink via autorun, Clink forwards Cmder's inject parameters to the already-injected Clink so that Cmder can work.
  • But I missed forwarding the profile directory (%cmder_root%\config). So Cmder starts properly now, but the profile directory is wrong, so Cmder's config file isn't found.

Fix in Clink: Forward the profile directory.

Issue in Cmder:

  • The cmder\vendor\clink.lua script is relying on using Cmder's profile directory. It's very easy and reasonable in Clink for a user to override the profile directory (though in the reported case here it's only accidentally overridden due to a bug in Clink).
  • So if/when any user intentionally overrides the profile directory, it will break the Cmder extensions to Clink and cause similar errors.
  • Issue when upgrading Cmder: Relying on the config file existing means that adding any other settings to the config file is very difficult to do without overwriting any customizations users have made.

Fix in Cmder: Add default logic to not rely on the cmder_prompt_config.lua file.

Then there's no need for Cmder to initially create the cmder_prompt_config.lua file (but it could create a placeholder file named differently e.g. with a .default extension that contains sample settings and instructions how to copy the placeholder file to make a real file). And if Cmder no long has to create/update the cmder_prompt_config.lua file then that solves the upgrade issue.

There are several approaches for adding default logic:

  1. At the top of clink.lua it could simply say uah_color = "\x1b[30m" (etc). And then when Clink loads the cmder_prompt_config.lua file afterwards, the config file would override the initial default. This is the simplest, but is not as safe as option 2.
  2. Another approach could be to use a function e.g. clink.lua line 90 could say prompt = get_uah_color() .. etc and make the get_uah_color() function be something like local function get_uah_color() return (uah_color or "\x1b[30m") end. That factors the logic into a single place and has the added bonus that if a script accidentally sets uah_color = nil then it will fall back to using a default. It's a little safer.
  3. Or if a variable is used only once, it might be reasonable to embed a default inline such as prompt = (uah_color or "\x1b[30m") .. etc. That's less initial effort than option 2, but can get unwieldy.

@chrisant996
Copy link
Contributor

I've published Clink v1.2.5 that should fix this autorun-vs-Cmder issue.

@daxgames It's still worth addressing the Cmder part of the issue (see above).

@daxgames
Copy link
Member

daxgames commented May 3, 2021

Please test this

@andyc56-mso
Copy link
Author

Please test this

Looking good! I also tried a Clink 1.2.5 manual install with build 1114 and that combo fixed the reported bug too. I see this latest one, build 1119, has Clink 1.2.5 with it as well (at least, according to the .cmderver file in %cmder_root%\vendor\clink). Both scenarios (build 1114 with manual install of Clink 1.2.5 and 1119 "straight up") look good.

@daxgames
Copy link
Member

daxgames commented May 3, 2021

Cool

@daxgames daxgames closed this as completed May 3, 2021
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

No branches or pull requests

3 participants
-