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

CSV export uses commas as thousands separator #963

Closed
Anarchid opened this issue Jul 13, 2022 · 8 comments · Fixed by #967
Closed

CSV export uses commas as thousands separator #963

Anarchid opened this issue Jul 13, 2022 · 8 comments · Fixed by #967
Assignees
Labels
⚡ Type: Bug Something isn't working

Comments

@Anarchid
Copy link

Mainsail Version:

2.2.0

Browser:

Firefox

Device:

Laptop

Operating System:

Linux

What happened?

The CSV export option in printer history seems to use , as a thousands separator, which causes the numerical value to be broken into two columns, and the export invalid.

For example, a print that expended 22843 millimeters of filament will be exported as 22,843.

Doing some RTFS, it seems that the exporter is supposed to switch to using semicolons as column separator in such case; however, this also does not occur, and any attempt to import the produced CSV is doomed to parse 22,843 as two separate columns.

What did you expect to happen instead?

CSV export should do one of these:

  • never use thousands separator when formatting numbers
  • always use semicolons as rows separator
  • use semicolons as rows separator when the exported file would contain even one numerical value >= 1000.

How to reproduce this bug?

Export history as CSV where any print exceeds 1000 mm of printed filament.

Additional information:

I suspect that this is probably due to browser locale settings or some such nonsense.

@Anarchid Anarchid added the ⚡ Type: Bug Something isn't working label Jul 13, 2022
@meteyou
Copy link
Member

meteyou commented Jul 13, 2022

pls upload a test export

@Anarchid
Copy link
Author

print_history.csv

@Anarchid
Copy link
Author

On some further thought, it looks like any numerical values are fed through toLocaleString here, and then not checked for the presence of escapeChar like in case of string below.

@meteyou
Copy link
Member

meteyou commented Jul 13, 2022

i use "toLocalString" because CSV in some countries have ; to split columns. for example in german we use . for thousand separator instead of , and , for decimal point. thats why CSV files are different in some countries.

@Anarchid
Copy link
Author

Hmm, perhaps an addiitonal thousands separator check like here with a switch to semicolon if , detected as thousands separator would be the best approach then.

@drewmo
Copy link

drewmo commented Jun 29, 2023

@meteyou This issue seems to have cropped up again when exporting print history. There is an unescaped comma in the start_time value.

You can see an example of this below:

image

And when viewing the exported csv in VScode:

Screenshot 2023-06-28 at 10 39 58 PM

The comma is not escaped in the start_time cell which is splitting the month / day value and year / time value into two columns.

@meteyou
Copy link
Member

meteyou commented Jun 29, 2023

@drewmo it's not the same issue. Only similar...

Which location / language do you use on your pc? Mainsail use the local time format from your pc, so I need this setting to reproduce the issue.

@drewmo
Copy link

drewmo commented Jun 29, 2023

@meteyou I am on a Mac, using the following date/time settings:

image

Also using Firefox as my browser with English(US) as the language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡ Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants