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

clink.lua updates #2860

Closed
wants to merge 8 commits into from
Closed

clink.lua updates #2860

wants to merge 8 commits into from

Conversation

daxgames
Copy link
Member

@daxgames daxgames commented Jul 9, 2023

Changes from #2856 and #2859

Adds

  • `%cmder_root%\vendor\clink.lua
    • Comments to explain some of less than clear purpose of some code.
    • Set global variable CMDER_CONFIG_PATH=%CMDER_ROOT%\config.
    • Set global variable CMDER_USER_CONFIG_PATH to the value of env var '%CMDER_USER_CONFIG%' IF is is set else nil.
    • Wrap gsub args 2 and 3 with verbatim()

@daxgames daxgames changed the title Clink updates clink.lua updates Jul 9, 2023
@daxgames
Copy link
Member Author

daxgames commented Jul 9, 2023

@chrisant996 I am not a lua dev so be gentle. :-)

I think I captured what you were talking about #2859 and #2856.

---
-- Global variable so other Lua scripts can detect whether they're in a Cmder
-- shell session.
---
CMDER_SESSION = true
CMDER_CONFIG_PATH = clink.get_env('CMDER_ROOT')..'\\config'
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant factor the environment variables in init.bat, not in clink.lua.

end

local function percent_prompt_filter()
clink.prompt.value = string.gsub(clink.prompt.value, "{percent}", "%%")
clink.prompt.value = string.gsub(clink.prompt.value, verbatim("{percent}"), verbatim("%%"))
Copy link
Contributor

Choose a reason for hiding this comment

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

The %% was already escaped.
Using verbatim("%%") turns into "%%%%", i.e. two percent signs.
The goal was one percent sign.
So this would need to be verbatim("%").

But even safer and simpler would be this:

clink.prompt.value = gsub_plain(clink.prompt.value, "{percent}", "%")

After adding the gsub_plain() function I was suggesting:

local function gsub_plain(str, find, replace)
    return string.gsub(str, verbatim(find), verbatim(replace))
end

And then people can use gsub_plain() instead of string.gsub(). And then it's simple to use, and simple to review. Any time a call to string.gsub() is seen being added, it's easy to review it and assess whether the intent is a plain string replacement, or a Lua pattern replacement (which is similar to regular expressions, but not quite the same). Any plain string replacements should use gsub_plain() instead.

-- If Cmder is launched with '/c [folderPath]' indicating Cmder is installed globally and
-- each user has a private '[folderPath]\config' folder.
---
CMDER_USER_CONFIG_PATH = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant to set environment variables in init.bat, and then in clink.lua use the environment variables.

Setting global Lua variables isn't needed, and introduces the opportunity for the global Lua variables and the environment variables to get out of sync (making troubleshooting very hard).

@@ -532,13 +543,13 @@ local function git_prompt_filter()
color = colors.conflict
end

clink.prompt.value = string.gsub(clink.prompt.value, "{git}", " "..color.."("..verbatim(branch)..")")
clink.prompt.value = string.gsub(clink.prompt.value, verbatim("{git}"), verbatim(" "..color.."("..verbatim(branch)..")"))
Copy link
Contributor

Choose a reason for hiding this comment

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

That nests calls to verbatim(), and it will double-escape the percent signs, which will mess them up.

It's one of the reasons I'm suggesting to create a gsub_plain() function. Then you don't have to manually figure out the right syntax for every separate gsub calling location.

Then you can write the lines in a much more intuitively readable manner.

clink.prompt.value = gsub_plain(clink.prompt.value, "{git}", " "..color.."("..branch..")")

@@ -577,13 +588,13 @@ local function hg_prompt_filter()
end

local result = color .. "(" .. branch .. ")"
clink.prompt.value = string.gsub(clink.prompt.value, "{hg}", " "..verbatim(result))
clink.prompt.value = string.gsub(clink.prompt.value, verbatim("{hg}"), verbatim(" "..verbatim(result)))
Copy link
Contributor

Choose a reason for hiding this comment

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

This nested verbatim calls. That will mess things up.

Copy link
Contributor

Choose a reason for hiding this comment

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

            clink.prompt.value = gsub_plain(clink.prompt.value, "{hg}", " "..result)

Copy link
Contributor

Choose a reason for hiding this comment

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

And so on, for the other string.gsub() calls further below.

@@ -153,7 +164,7 @@ local function set_prompt_filter()
end

if prompt_useHomeSymbol and string.find(cwd, clink.get_env("HOME")) then
cwd = string.gsub(cwd, clink.get_env("HOME"), prompt_homeSymbol)
cwd = string.gsub(cwd, verbatim(clink.get_env("HOME")), verbatim(prompt_homeSymbol))
Copy link
Contributor

Choose a reason for hiding this comment

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

cwd = gsub_plain(cwd, clink.get_env("HOME"), prompt_homeSymbol)

Comment on lines +190 to +193
prompt = string.gsub(prompt, verbatim("{uah}"), verbatim(uah))
prompt = string.gsub(prompt, verbatim("{cwd}"), verbatim(cwd))
prompt = string.gsub(prompt, verbatim("{env}"), verbatim(env))
clink.prompt.value = string.gsub(prompt, verbatim("{lamb}"), verbatim(prompt_lambSymbol))
Copy link
Contributor

Choose a reason for hiding this comment

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

    prompt = gsub_plain(prompt, "{uah}", uah)
    prompt = gsub_plain(prompt, "{cwd}", cwd)
    prompt = gsub_plain(prompt, "{env}", env)
    clink.prompt.value = gsub_plain(prompt, "{lamb}", prompt_lambSymbol)

return false
end
end

-- No git present or not in git file
clink.prompt.value = string.gsub(clink.prompt.value, "{git}", "")
clink.prompt.value = string.gsub(clink.prompt.value, verbatim("{git}"), verbatim(""))
Copy link
Contributor

Choose a reason for hiding this comment

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

    clink.prompt.value = gsub_plain(clink.prompt.value, "{git}", "")

-- 'CMDER_CONFIG_PATH` dir, so we need to load scripts from there before Clink loads lua
-- scripts from the profile directory given to it when it was injected.
---
if not (isempty(CMDER_USER_CONFIG_PATH)) then
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend to add the comment, but don't change the code to use global Lua variables. Using environment variables here is good.

I was recommending to add another environment variable in init.bat such as set CMDER_DEFAULT_CONFIG_DIR=%CMDER_ROOT%\config, and then use %CMDER_DEFAULT_CONFIG_DIR% in all the places instead of %CMDER_ROOT%\config.

@daxgames daxgames closed this Jul 10, 2023
@daxgames
Copy link
Member Author

daxgames commented Jul 10, 2023

WOW! I thought I understood, did not know it was possible to be so wrong. :-)

Closed this and will revisit.

@chrisant996
Copy link
Contributor

No worries! 😁

@chrisant996
Copy link
Contributor

@daxgames Oh, tell you what -- I will make a PR for the verbatim part.

While working on the git_prompt.lua change to use async prompt filtering, I realized there's more that needs to happen for the verbatim part. In the replace param only % needs to be escaped, but in the find param several other characters also need to be escaped.

I'll make a fix for git_prompt.lua slowness in clink-completions, and then I'll make a PR for the verbatim change in cmder.

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