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\clink.bat" inject " unexpected at this moment #571

Closed
alfablac opened this issue Mar 7, 2024 · 16 comments
Closed

\clink\clink.bat" inject " unexpected at this moment #571

alfablac opened this issue Mar 7, 2024 · 16 comments
Labels
bug Something isn't working

Comments

@alfablac
Copy link

alfablac commented Mar 7, 2024

Latest update clink.1.6.7.87398b breaks clink, had to rollback to clink.1.6.6.87eebc
When running either via clink.bat or the injected cmd, it throws this error on title.

@rybnicek
Copy link

rybnicek commented Mar 7, 2024

Happens to me as well on both W10/W11 x64 in cmd shell. PowerShell seems unaffected.

@vii1
Copy link

vii1 commented Mar 7, 2024

There's a superfluous double quote in clink.bat line 34. I removed it and it works.

Before:

    start "Clink" cmd.exe /s /k ""%~dpnx0" inject %clink_profile_arg%%clink_quiet_arg%"

After:

    start "Clink" cmd.exe /s /k "%~dpnx0" inject %clink_profile_arg%%clink_quiet_arg%"

@chrisant996
Copy link
Owner

Latest update clink.1.6.7.87398b breaks clink, had to rollback to clink.1.6.6.87eebc When running either via clink.bat or the injected cmd, it throws this error on title.

@alfablac Thanks for reporting this.

I am completely unable to reproduce the problem.
It seems there must be more to it than just running clink.bat -- maybe there is some Windows configuration that matters as well.

There's a superfluous double quote in clink.bat line 34.

@vii1 Count the quotes carefully. It's not superfluous, and it didn't change.

What changed is the line got moved inside a ( .. ) pair.
Apparently, that is changing how CMD parses the quotes, but only on some computers.

Strangely, I cannot reproduce the problem at all (which is how the problem got published).

Happens to me as well on both W10/W11 x64 in cmd shell. PowerShell seems unaffected.

@rybnicek Yes, it's expected that PowerShell would be unaffected by anything in Clink, because Clink has nothing to do with PowerShell.

@chrisant996
Copy link
Owner

chrisant996 commented Mar 7, 2024

Can you try the following clink.bat file instead?

EDIT: removed; use updated Clink instead.

@vii1
Copy link

vii1 commented Mar 7, 2024

@vii1 Count the quotes carefully. It's not superfluous, and it didn't change.

You're right. In fact my attempt broke running from the start menu icon. I tried escaping the inner quotes with ^" and it fixes autoinject but running from icon shows "The system cannot find the specified path" (but runs Clink anyway)

@chrisant996 chrisant996 added the bug Something isn't working label Mar 7, 2024
@chrisant996
Copy link
Owner

I can't reproduce the problem as described, but I think I see what's causing it.
It looks like the described repro steps are either ambiguous or not quite accurate.

@chrisant996
Copy link
Owner

I can reproduce a little different error when running clink.bat startmenu or when double clicking on the Clink.lnk in the Start Menu after using the setup exe installer.

Those aren't the same as "running either via clink.bat or the injected cmd", so maybe the described repro steps were phrased ambiguously?

I have a fix, though.

@mkurup27
Copy link

mkurup27 commented Mar 7, 2024

Can you try the following clink.bat file instead?

[ removed the quoted test bat file so no one tries to use it; use v1.6.8 instead ]

I just installed clink and encountered the same issue. I tried this clink.bat and it works fine now.

@chrisant996
Copy link
Owner

@mkurup27 Don't use that clink.bat. It solves one problem but causes another under other circumstances. v1.6.8 is coming in a few minutes.

@ssomers
Copy link

ssomers commented Mar 7, 2024

I saw it too and 1.6.8 fixes it, so I'm good. But FYI: I never "run" clink, know about clink.bat or configured anything; I just run the installer and open up a console (or run cmd.exe). So I wonder what the circumstances are: is there something peculiar about my registry or settings:? I don't see anything extraordinary in %HOME%\AppData\Local\clink\clink_settings.

@chrisant996
Copy link
Owner

@ssomers I don't know, and the mystery is annoying me. I was able to reproduce a different variation of the problem, and that enabled me to verify a fix. But I'm completely unable to reproduce the described problem on W10 or W11 (x64 on both). Even setting up AutoRun for Clink doesn't reproduce it.

But the underlying problem was I had broken subtle cooperation between shift and call in the script. If any of the if conditions in the clink.bat file end up running shift, then the later %~dp0 usages don't refer to the batch script name. The call :launch is important because a side effect of call is that %0 gets reset to be the batch script name again despite any preceding shift.

And in fact there is still a problem with that: running clink --profile c:\temp\test_profile inject fails (with a different error message). But that problem has existed since before I got involved with Clink at all -- it even exists in Clink v0.4.9.

The part I don't understand is how people were hitting this shift-related problem when using a syntax that doesn't run any of the shift commands? Maybe the executed command wasn't literally clink.bat or clink inject, or maybe there is some other system level configuration (not Clink configuration) that is somehow relevant, or maybe there were two separate problems (but then why couldn't I reproduce it?).

@ssomers
Copy link

ssomers commented Mar 7, 2024

Well, I've done enough cmd to not dare to try doing what clink.bat does. If you run this .bat in a regular directory:

(
    echo Hello, %~p0!
)

it works as expected. But copy it to c:\Program Files (x86)\clink and the mysterious message appears. It's only the ~p part that does it, so it's probably the closing brace after x86 that gets misinterpreted.

PS Actually, you just need to rename your working directory to so that it contains a closing brace - there's no different interpreter for scripts under c:\Program Files. Well, at least this example doesn't demonstrate that.

@chrisant996
Copy link
Owner

chrisant996 commented Mar 7, 2024

Well, I've done enough cmd to not dare to try doing what clink.bat does. If you run this .bat in a regular directory:

(
    echo Hello, %~p0!
)

it works as expected. But copy it to c:\Program Files (x86)\clink and the mysterious message appears. It's only the ~p part that does it, so it's probably the closing brace after x86 that gets misinterpreted.

Ah! Mystery solved! Thanks for sharing the observation.

There were two problems.

  1. The cooperation between shift and call.
  2. The use of (..) combined with the Clink program directory being under Program Files (x86) AND the (correct and necessary) use of "" before the %~dp0, which has a side effect of preventing CMD from treating the embedded ) as being quoted in the first parsing pass.

Mine is under c:\wbin\clink, thus I could not reproduce the problem.

@chrisant996
Copy link
Owner

The clink.bat implementation is juggling many syntax and chipset and performance (speed) details all at once. It's indeed challenging to keep all of them working in proper harmony.

@mkurup27
Copy link

mkurup27 commented Mar 8, 2024

@chrisant996, thanks for the heads-up. It has been working fine for me but I've installed the latest version anyways. Thanks!

@alfablac
Copy link
Author

I just noticed the update and I forgot I reported this. haha Thanks for fixing it @chrisant996 !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants