-
Notifications
You must be signed in to change notification settings - Fork 220
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
Invalid escaping of {staged_files} #786
Comments
Hey! Thank you for submitting this issue. I couldn't reproduce the issue on my macOS, so I assume this is something OS-related. I will try to figure it out. |
From my perspective it looks like git command returns the file list in different encoding. Could you try setting the default encoding for Powershell to UTF-8 (if not set yet)? I found some useful options here: https://stackoverflow.com/questions/57131654/using-utf-8-encoding-chcp-65001-in-command-prompt-windows-powershell-window/57134096#57134096 |
It is already on utf8 code page (65001). Which git command lefthook runs? Can I "test" the single comment to see how it behaves? |
Yes, it's |
Maybe this can help: https://stackoverflow.com/questions/10651975/unicode-utf-8-with-git-bash |
Ok, found a workaround.
Returns:
Restoring the config
Returns:
I close the issue. It's not related to lefthook, but to git actually. Thanks for the help |
Sorry, I reopen the issue. The git config I set remove the escaping, but also the surrounding quote, which breaks lefthook Without With This leads to lefthook error:
I'm not sure how lefthook works internally, but I wonder if it run a git command and analyses the results, or if it uses any package. If it works using the former, maybe there's something to do to be more reliable ? |
I don't know if it can solve the issue, but git diff has a -z flag that does not encode the file name:
The final char is a NUL char. So what about adding the flag to lefthook, take the output minus the last NUL char and adding surrounding quotes? |
Could you try wrapping the pre-commit:
commands:
lint:
run: yarn lint "{staged_files}" Of single quotes if double quotes don't work. This quoting should wrap each filename with the quotes. |
If I both wrap the However, I don't feel comfortable with this solution. Git options are not committed as part of the source code. It requires every developer to set the option, and will eventually lead to issues or conflict with other tools that may rely on git commands (I'd bet turborepo or NX are using same technics to detect affected projects). I never works with go and I have no idea of the impact/complexity, but I'd wish lefthook take the normalization into account. From the git doc
Maybe there's some lib in go that can revert the normalization back to plain utf8 encoding ? (🤫 https://chatgpt.com/share/448c4b2f-c19d-4532-a57c-c7c0d9573a44) This would create a bullet proof and cross platform way to read the file names. |
For the last issue the problem is that Windows doesn't work well with single quotes (as far as I know), and considers But I like your idea for normalization of the output. It makes sense. I'll put it on my radar 👍 |
I've also bumped into an issue with wrong escaping of If file name contains square brackets ( |
@nicky1038 , in this case use |
@mrexox Thank you for the quick response, the suggested solution works like a charm! And sorry for not reading docs before asking questions 😶 |
🔧 Summary
If file names contains some utf8 (I guess) characters, lefthook fails because of invalid escaping.
(part of the message is in French, probably the system error lefthook trapped).
Lefthook version
1.7.7
Running on Windows 10. Both reproed using PowerShell 7 and legacy cmd console.
Steps to reproduce
Clone this repo: https://github.com/stevebeauge/repro-lefthook-wrong-escape
Edit the file
files/KO é.json
. Set whatever you want in the file. If you are using a powershell console you can runThen try to commit the changes:
Expected results
It should work whatever the file name is (assuming it's a valid filename for the system).
Actual results
It fails because utf8 character is escaped, but the escape character is itself not properly escaped:
Possible Solution
Logs / Screenshots
Running
pnpm lefthook run pre-commit -v
in a powershell console generates this logs:As a comparison, modifying
files/ok.json
is working. It's verbose output is:The text was updated successfully, but these errors were encountered: