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: Use C locale in diagnostics #1481

Merged
merged 1 commit into from
Jul 18, 2023
Merged

fix: Use C locale in diagnostics #1481

merged 1 commit into from
Jul 18, 2023

Conversation

buhtz
Copy link
Member

@buhtz buhtz commented Jul 18, 2023

C locale is always used when extracting version strings from external tools in diagnostics output.

In addition: Renamed "match" variable because it is a reserved keyword in newer Python versions.

Related #1479

@aryoda
Copy link
Contributor

aryoda commented Jul 18, 2023

I am not sure if setting LC_ALL will really work (at least I had a lot of problems with that with R).

If do a quick test the other way around it does not work but still print English output on my machine (U22.04):

env LC_ALL=de_DE.UTF-8 rsync --version

Perhaps it works only with C as value...

@buhtz
Copy link
Member Author

buhtz commented Jul 18, 2023

On a bash I have to do export LC_ALL= instead of env LC_ALL=. Don't know why "env" does not work. I was using env wrong in the past using && to connect it with a command: env LC_ALL=C && encfs is of course wrong.

I installed italian local on my machine (via sudo dpkg-reconfigure locales on Debian 11). And then I checked it inside a Python REPL using the Popen(cmd, env=...). This worked fine.

@aryoda
Copy link
Contributor

aryoda commented Jul 18, 2023

On a bash I have to do export LC_ALL= instead of env LC_ALL=.

Good to know, THX for testing!

I have justs asked the user of the underlying issue to to a test with env, he could also do test the without env to be sure that at least one variant is working on his computer...

@aryoda
Copy link
Contributor

aryoda commented Jul 18, 2023

Good work, ready to merge!

OP did successfully test that LC_ALL=C works (with or without env doesn't matter here, since in Python we use popen: #1479 (comment)

@buhtz buhtz merged commit 71c9657 into bit-team:dev Jul 18, 2023
@buhtz buhtz deleted the fix/1479diagnostics branch July 18, 2023 14:33
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