Skip to content

Conversation

chrisant996
Copy link
Contributor

Hi, I'm the maintainer for clink.

I'd like to update z.lua with the following:

  • Update the auto-init in z.lua to fix z.cmd, esp. the HomeDir environment variable so the Lua engine can find the z.lua script.
  • Update the completions for z.lua in Clink, to include description strings and to add directory completions for the -x flag.

The auto-init for z.cmd was missing a couple of lines.  The HomeDir
line, in particular, is important because it enables Lua to find the
z.lua script.
- Added description strings for the flags.
- Added dir completions for the `-x` flag.
@skywind3000 skywind3000 merged commit 4d89b55 into skywind3000:master May 17, 2023
@skywind3000
Copy link
Owner

thanks

@skywind3000
Copy link
Owner

skywind3000 commented May 18, 2023

I have a question about "HomeDir" environment variable,

in this line:

z.lua/z.lua

Line 2727 in 4d89b55

print('set "LuaScript=' .. os.scriptname() .. '"')

the absolute path name of z.lua has already been stored in LuaScript, and z.lua will be called as LuaScript in
the other parts of the batch file, like:

z.lua/z.lua

Line 2572 in 4d89b55

call "%LuaExe%" "%LuaScript%" -h

why do we still need the "HomeDir" to tell lua how to find z.lua script, since the absolute path is given.

I am trying to google "HomeDir" + "Lua" , but can't find anything, it is supported by lua interpreter right ?

Is there any other side-effect if we change it ?

@chrisant996
Copy link
Contributor Author

I have a question about "HomeDir" environment variable,
...
why do we still need the "HomeDir" to tell lua how to find z.lua script, since the absolute path is given.

@skywind3000 You're right; currently, those two lines don't do anything. I was trying to make --init match the z.cmd file in the repo, but I missed that the next two lines also don't match.

Why does z.cmd have these 4 lines, but the z.cmd generated by --init uses a different approach with a hard-coded path?

Would it be reasonable to make z_windows_init() use the same dynamic approach as in the pre-generated z.cmd file, instead of the hard-coded approach? The dynamic approach doesn't have to run any extra code to find the script, as long as z.cmd and z.lua are located in the same directory.

Or are they intentionally different? Is the intent to normally use the z.cmd from the repo, and to only use --init if the z.cmd and z.lua files will be moved into separate directories?

@skywind3000
Copy link
Owner

skywind3000 commented May 18, 2023

The generated version is designed to be more robust,

  • it can locate lua interpreter even if the "lua.exe" is not in %PATH% (by using an absolute path name of lua)
  • it can be put to the different place, not required to reside in the same directory of z.lua.

every generated init code (including bash, zsh, fish and powershell) uses LuaExe and LuaScript to store
absolute path of lua executable and the z.lua script.

To keep consistency, the generated cmd code should not be an opposite sample.

But, for convenience and portability, I provide a separated batch, it will try to find z.lua in the same directory
and try to call lua in %PATH%. In some cases, it just enough and can be easily distributed.

For example, the two files can be compressed in an zip file, and extracted to where ever you want, just keep
them in the same directory and provide a lua interpreter in %PATH%, no extra step needed.

That's why I put a separated z.cmd in the repo.

@skywind3000
Copy link
Owner

skywind3000 commented May 18, 2023

z.lua/z.cmd

Lines 4 to 7 in 4d89b55

set "HomeDir=%~dp0"
set "PathSave=%PATH%"
set "LuaExe=lua"
set "LuaScript=%HomeDir%z.lua"

Here we can see, HomeDir is a temporary variable to calculate LuaScript, has no reference in other parts of z.cmd
once we get LuaScript, we don't need it anymore.

So HomeDir is not necessary in the generated z.cmd, since LuaScript is already initialized by the lua script.

@chrisant996
Copy link
Contributor Author

Here we can see, HomeDir is a temporary variable to calculate LuaScript, has no reference in other parts of z.cmd once we get LuaScript, we don't need it anymore.

So HomeDir is not necessary in the generated z.cmd, since LuaScript is already initialized by the lua script.

@skywind3000 I'll make another PR to back out the two lines from the --init code since they're unnecessary in the generated file.

Are you asking me to also edit the provided z.cmd file to remove line 4 and replace the %HomeDir% in line 7 with %~dp0?

Also, is the PathSave variable needed? I can't see where it's used; it seems to be the only reference to PathSave in the whole repo. Do you want me to remove it as well from the provided z.cmd file?

@skywind3000
Copy link
Owner

Since we have the agreement about how to modify the batch file.
Who will make the patch is not important.

Right now, I have time and I made the patch:

71bae7f

@chrisant996
Copy link
Contributor Author

Awesome, thanks for asking the question, and for cleaning it up!

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