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_start.cmd is run twice when clink is installed in %localappdata%\clink #653

Closed
hutcho opened this issue Aug 9, 2024 · 11 comments
Closed
Labels
enhancement New feature or request low priority Low priority

Comments

@hutcho
Copy link

hutcho commented Aug 9, 2024

clink_start.cmd gets executed twice if you have installed clink into %localappdata%\clink on Windows 10. I have tested the same steps below on different paths, and the issue only occurs when clink is installed at %localappdata%\clink.

Steps to reproduce:

  1. Download clink .zip and extract to %localappdata%\clink
  2. Install clink by running clink autorun install from the above folder
  3. Create a clink_start.cmd file e.g. echo doskey e = explorer $* > clink_start.cmd
  4. Close all command prompts, and start a new command prompt.

Result shown in terminal

<some text above>
https://github.com/chrisant996/clink


doskey e = explorer $*

doskey e = explorer $*

>

It is unexpected to see doskey e = explorer $* appear twice on terminal startup.

I noticed clink info shows 2 identical entries on the scripts line:

scripts: C:\Users\USERNAME\AppData\Local\clink ; C:\Users\USERNAME\AppData\Local\clink

But using clink set clink.path C:\Users\USERNAME\AppData\Local\clink does not stop the double running of the .cmd file.

@chrisant996
Copy link
Owner

You've installed Clink into its own profile directory. That's not something that was considered as something that might ever be attempted. I guess I can understand why someone could choose to do that, though.

As an analogy, it's similar to installing Windows into C:\users\myname instead of C:\Windows.

The Startup Cmd Script documentation says:

When Clink is injected, it looks for a clink_start.cmd script in the binaries directory and profile directory. Clink automatically runs the script(s), if present, when the first CMD prompt is shown after Clink is injected and before any Lua scripts run. You can set the clink.autostart setting to run a different command, or set it to "nul" to run no command at all.

The program directory (the "binaries" directory) and the profile directory were meant to be separate directories.
In your case, they're the same directory, so when Clink checks for clink_start.cmd in each of the directories, it finds one in each, and executes both of them.

I can make it check whether the two clink_start.cmd files are the same file, and if so then only run it once.

You're welcome to experiment with putting the binaries in the profile directory, but you might run into other strange quirks as well. It's not an intended or tested way of using Clink.

@chrisant996 chrisant996 added enhancement New feature or request low priority Low priority labels Aug 9, 2024
@hutcho
Copy link
Author

hutcho commented Aug 9, 2024

Ah thankyou that makes sense if it's unsupported. Since the .zip is offered as a way to install clink, could there be a recommended path for installing it to like in your example, it's recommended to install windows to c:\Windows. What would be the default expected location for clink?

@chrisant996
Copy link
Owner

Ah thankyou that makes sense if it's unsupported. Since the .zip is offered as a way to install clink, could there be a recommended path for installing it to like in your example, it's recommended to install windows to c:\Windows. What would be the default expected location for clink?

The difference is that we don't manually copy files out of a zip file to install Windows.

The Clink setup.exe installer is the recommended way to install the program, and it recommends installing to the place that Windows recommends programs be installed.

To me it feels presumptuous or insulting to recommend where to copy a program, if the user has already made the choice to avoid using an installer program.

(Windows doesn't recommend to manually install files into Program Files; that's something that typically only installer programs should do. And %LOCALAPPDATA% is intended for use by programs for storing data and settings, which is part of why it's a hidden folder. Nothing will block a user from peeking into it or putting things in it, though.)

@hutcho
Copy link
Author

hutcho commented Aug 9, 2024

The problem with the Clink setup.exe installer is that it requires admin privileges, but the .zip doesn't. Therefore the choice to not use the installer will not always be a user choice if they dont have admin rights. They may wish they could use the installer but are forced to use the .zip. In this case, I think that specific user would appreciate guidance on an install location, and it wouldn't be an insult to them. We've already developed some guidelines here about where not to put it (e.g. not in %localappdata%/clink, and not in Program Files.)

@andry81
Copy link

andry81 commented Oct 31, 2024

@hutcho

3. Create a clink_start.cmd file e.g. echo doskey e = explorer $* > clink_start.cmd

Just a suggestion:

echo @doskey e = explorer $* > clink_start.cmd

@hutcho
Copy link
Author

hutcho commented Oct 31, 2024

@andry81 what is your suggestion trying to achieve?

@chrisant996
Copy link
Owner

@andry81 what is your suggestion trying to achieve?

The suggestion is to insert @ at the beginning of the clink_start.cmd file, to suppress the output.

The suggestion misunderstands the problem report, and actually ruins the ability to observe the reported problem.

In a real-world situation, then inserting a line @echo off is a better technique than prefixing every single line with @. But step 3 is part of a set of repro steps to observe the problem, and inserting @ would have made it impossible to observe the problem.

@andry81
Copy link

andry81 commented Oct 31, 2024

In a real-world situation, then inserting a line @echo off is a better technique than prefixing every single line with @

But there is only a single line (> rewrites the file), so no need to insert another one.

@chrisant996
Copy link
Owner

In a real-world situation, then inserting a line @echo off is a better technique than prefixing every single line with @

But there is only a single line (> rewrites the file), so no need to insert another one.

But you have completely misunderstood the purpose of the line -- it's absolutely important to not have a @, because it is a repro step to demonstrate that the script runs twice. You're trying to make the script print no output, but that would make it impossible to see that the script runs twice. The suggestion doesn't apply in this context.

I don't know how to explain it more clearly than has already been stated earlier.

@andry81
Copy link

andry81 commented Oct 31, 2024

I don't know how to explain it more clearly than has already been stated earlier.

May be because I didn't answer to the author's question, but answered to your remark.

@chrisant996
Copy link
Owner

I don't know how to explain it more clearly than has already been stated earlier.

May be because I didn't answer to the author's question, but answered to your remark.

In a real world script it's more beneficial to plan ahead: The chances are very low that a startup script will stay only one line.
Inserting @echo off is the standard practice because then no further special treatment is needed for any of the other lines that get added later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority Low priority
Projects
None yet
Development

No branches or pull requests

3 participants