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

feat: export print history as CSV #675

Merged
merged 11 commits into from
Feb 28, 2022
Merged

Conversation

meteyou
Copy link
Member

@meteyou meteyou commented Feb 24, 2022

This PR add a function to export selected or all jobs to an CSV file.

fix: #404

Signed-off-by: Stefan Dej [email protected]

@wlhlm
Copy link

wlhlm commented Feb 25, 2022

I see this just joins the strings of each job field with a semi-colon. Does that work safely with filenames that might contain reserved characters?

@pataar
Copy link
Member

pataar commented Feb 25, 2022

Note that the delimiter is different in the US and UK. EU: ';', USA/UK: ','

@meteyou
Copy link
Member Author

meteyou commented Feb 25, 2022

Does that work safely with filenames that might contain reserved characters?

i think that you would have problems with uploading or printing and that's why you wouldn't even see something like that in the history.

@meteyou
Copy link
Member Author

meteyou commented Feb 25, 2022

Note that the delimiter is different in the US and UK. EU: ';', USA/UK: ','

ohhh... thx for this info! because i just use US number format for the export, i think it will be fine to use ',' only.

@wlhlm
Copy link

wlhlm commented Feb 26, 2022

@meteyou

Does that work safely with filenames that might contain reserved characters?

i think that you would have problems with uploading or printing and that's why you wouldn't even see something like that in the history.

I have just tested this and indeed you can't currently print a GCode file with a semicolon in the name because Klipper treats everything following a semi-colon as GCode comment regardless of whether it is quoted or not.

However, the Klipper stack certainly works with commas in filenames as I've been using such a filename scheme for ages and that will result in broken CSV output when using a mainsail language that results in the comma field-separator as you've just implemented.

format numbers to locale strings and escape CSV seperator in strings

Signed-off-by: Stefan Dej <[email protected]>
@meteyou
Copy link
Member Author

meteyou commented Feb 27, 2022

@wlhlm thx for testing. i pushed a fix. with the new commit i use the local number format and escape the csv seperator in strings. pls test this again, if this would fit.

@wlhlm
Copy link

wlhlm commented Feb 27, 2022

Thx for the fix! I'm wondering why not pull in a battle-tested CSV generator as a dependency that does proper escaping instead of a home-grown implementation? Just skimming through my mainsail node_modules for example I could find d3-dsv which is an indirect dependency of mainsail. It's a pretty straight-forward module with about 160LOC.

@meteyou
Copy link
Member Author

meteyou commented Feb 27, 2022

Because it's easier and faster to generate it this way and I don't want to add new deps for it (the module you linked above, I think it is to parse a CSV file. Reverse direction I need in this case).

@wlhlm
Copy link

wlhlm commented Feb 27, 2022

(the module you linked above, I think it is to parse a CSV file. Reverse direction I need in this case).

Have a look at d3.csvFormat().

@meteyou
Copy link
Member Author

meteyou commented Feb 27, 2022

Have a look at d3.csvFormat().

this function do complete the same as i implemented. i have to define the seperator exactly the same way... its more complex to use a lib for this very very small usage. a CSV file is just a text file. if we would export an excel file, i would use a dep.

@meteyou meteyou merged commit 5d146e9 into develop Feb 28, 2022
@meteyou meteyou deleted the feat/export-print-history branch February 28, 2022 23:22
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.

[FR] Export Print History as .CSV
3 participants