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

Fix parsing of unicode filenames reported by git ls-files (#1339) #1340

Merged
merged 1 commit into from Feb 12, 2021

Conversation

phihos
Copy link
Contributor

@phihos phihos commented Feb 11, 2021

I fixed German umlauts in filenames by changing this

git ls-files '*.yaml' '*.yml'

to this

git ls-files -z '*.yaml' '*.yml'

The output in not delimited by newlines but by \0 instead. So I also had to change the output parsing.

A test case is included.

@phihos phihos force-pushed the fix-1339 branch 2 times, most recently from 953ac61 to fe71a8b Compare February 11, 2021 21:39
@phihos phihos changed the title Fix broken umlauts in filenames (#1339) Fix broken umlauts in file names (#1339) Feb 11, 2021
@phihos
Copy link
Contributor Author

phihos commented Feb 11, 2021

For the tests to pass with Python 3.6 I had to modify the test function run_ansible_lint to merge environment variables from the safe list to the _env var even if env in not None. Else variables like LANG are not passed and subprocess.call(...) tries to convert the unicode output to acii causing an error.

@phihos phihos changed the title Fix broken umlauts in file names (#1339) Fix broken umlauts in filenames (#1339) Feb 11, 2021
@ssbarnea ssbarnea self-requested a review February 12, 2021 09:17
@ssbarnea ssbarnea added the bug label Feb 12, 2021
@ssbarnea ssbarnea changed the title Fix broken umlauts in filenames (#1339) Fix parsing of unicode filenames reported by git ls-files (#1339) Feb 12, 2021
Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, it really make me happy to review a pull-request that fixes a bug while explaining what and why. I am hoping to see more from you.

@ssbarnea ssbarnea merged commit 65f4ef7 into ansible:master Feb 12, 2021
@phihos phihos deleted the fix-1339 branch February 12, 2021 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants