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

Bug 799454 - Numeric value in exported CSV transactions #2044

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

christopherlam
Copy link
Contributor

No description provided.

@gjanssens
Copy link
Member

What decimal separator will this output for the numeric value ? Can our CSV import code still import the exported file with this change (and have you tested this in multiple locales) ?

@christopherlam
Copy link
Contributor Author

christopherlam commented Nov 2, 2024

As usual @gjanssens is spot on. LANG=fr_FR.utf8 leads to numbers exported as "2002,34" instead of the defacto 2002.34. Should we use printf("%.8f", number); instead?

@gjanssens
Copy link
Member

My point really is you can't expect the numbers to be completely locale independent. It would be pretty user-unfriendly to enforce a dot as decimal separator globally. There are plenty locales where numerical data uses a comma as decimal separator. For example here in Belgium, when I receive csv data from say my bank, it will have numbers using comma as decimal separator. And I'd expect the csv file exported from Gnucashe to show numbers in my locale.

So in fact I think there is no need to change the current (default) behavior really.

But if you must, I can see it may be useful to remove the thousands separators as not all applications other than gnucash are as flexible during import and might choke on those.

Exporting data to import it again in another locale is IMO a less common scenario than exporting and importing in the same locale. It would be unfortunate if we would force this less common behaviour on everybody whose locale doesn't have a dot as decimal separator.

If it turns out many people do export/import to different locales, the best thing to do would be to provide a locale option during export (which default to current locale).

@jralls
Copy link
Member

jralls commented Nov 2, 2024

Maybe we should get a little more information from the bug reporter about what he's trying to do and why the thousands separator is causing a problem.

Note that there is a locale-free way to represent numbers: Fractional notation. In a CSV that would be =12345/100 with the = indicating that the field contains a formula. That will work for importing into a spreadsheet but other CSV consumers might not be able to interpret it correctly.

Mind, I'm in favor of removing thousands separators from CSV numeric values regardless. Thousands separators are a aid for humans to interpret a number but a distraction for computers. CSV data is intended to be read by computers, not humans.

@gjanssens
Copy link
Member

I agree both on asking the user what problem he actually has and removing the thousands separators. I think using fractional notation would make our csv export less compatible to other csv consumers so I'd not go that route.

@jralls
Copy link
Member

jralls commented Nov 3, 2024

I asked the user, he's having trouble with importing into LibreOffice with the thousands separators so we don't need to do any locale gymnastics.

As usual @gjanssens is spot on. LANG=fr_FR.utf8 leads to numbers exported as "2002,34" instead of the defacto 2002.34. Should we use printf("%.8f", number); instead?

No, GncCommodityPrintInfo has a use_locale bit that you disabled. Leave it alone and just setuse_separators to FALSE.

@jralls
Copy link
Member

jralls commented Nov 4, 2024

OK, commit when ready.

@code-gnucash-org code-gnucash-org merged commit 764157d into Gnucash:stable Nov 4, 2024
4 checks passed
@christopherlam christopherlam deleted the bug799454 branch November 4, 2024 03:47
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.

4 participants