-
Notifications
You must be signed in to change notification settings - Fork 772
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
Set classic locale for PDF export link attributes #3551
Conversation
Added some test and meaningful return values to detect if the
This PR is ready for review. Testing on different platforms is necessary, as locale related things tend to be messy. |
/azp run create-installers |
Azure Pipelines successfully started running 1 pipeline(s). |
(Ah, the Debian build fails with |
Please rebase once more. This is already fixed on master (#3547)For release-1.1 you'll have to cherry-pick those commits. |
/azp run create-installers |
Azure Pipelines successfully started running 1 pipeline(s). |
Confirm the fix on Manjaro Linux. Thanks! |
Thanks! I cherry-picked. I'll remove the commit before this is merged. |
We actually want this commit on release-1.1. So please cherry-pick this directly on the release branch and push it to the repository. |
The requested installers were generated successfully. The installers are available at: Please be aware that these links are only valid for a limited time (~30 days). |
Done. |
I have tested the generated installers on Windows 10 and MacOS Catalina, exporting the pdf from the poppler issue and checking the exported file. |
Thanks for testing! I made sure we get an error popup when the pdf export fails, and I added some debug info in the terminal output. Could you get me what debug output you get? It looks like a bunch of this:
It'd be interesting to also get this output on Windows and, say, Ubuntu (or any other distrib where the export worked before this PR). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Here is a simpler test pdf, which also has utf8 strings in label names: |
/azp run create-installers |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I found the source of the original problem (see this post). |
This comment has been minimized.
This comment has been minimized.
On Windows 10 with Portaudio 19.07 the export works fine (tested with
|
A lot confuses me in this log...
then
So even if Then, if the C locale is I'll add more precise outputs to try and figure things out. |
I realized something:
With any other locale than On the other hand, when I think the proper solution to the problem at hand is:
However, the C locale is use by Any opinion? |
Yes, indeed, floating point numbers in
By the way in Switzerland it depends on a lot of factors whether people use a comma or a decimal point. Personally I use a decimal point (but still pronounce it as |
Beware that your test only works if you do have the locale generated on your machine (and apparently you don't have
because I don't have
so commas, probably from another canton :-) |
I must insist that decimal points are used in German speaking Switzerland: Contrast to Germany: I already had the |
On Windows 10 after commit da49fbc
|
On Ubuntu 21.10 after commit da49fbc (export works):
|
On macOS Catalina (export works now):
|
@rolandlo Thanks a lot for your patience and testing! I removed the debug outputs. This PR is now ready for review (provided the checks clear). |
Azure Pipelines successfully started running 1 pipeline(s). |
The requested installers were generated successfully. The installers are available at: Please be aware that these links are only valid for a limited time (~30 days). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Merging in 24h if no objection is raised |
Fixes #3388