Desktop: Change 'CSV summary dive details' output from TSV to CSV.

[mikeller]

Describe the pull request:

• Bug fix
• Functional change
• New feature
• Code cleanup
• Build system change
• Documentation change
• Language translation

Pull request long description:

Change the output formato for the Export / 'CSV summary dive details' from TSV to CSV, to make it consistent with the menu item name, and with the other 'CSV' export function.
This was changed to TSV by @mturkia in
6c82578, but I could not find any discussion as to why.
Also removed replacement of the field separator in any fields, as, according to the CSV RFC (https://datatracker.ietf.org/doc/html/rfc4180) this is not required as long as the fields containing separators are enclosed in quotes, and any quotes within the fields are doubled up. Tested with a fairly large log file, and importing into Google Sheets is working fine for the output produced.
Also made capitalisation of the Export menu items consistent.

Changes made:

1. change the field separator for 'CSV summary dive details' from tab to comma;
2. remove the replacement of the field separator in fields in 'CSV summary dive details';
3. make the capitalisation of menu entries in the 'Export' menu consistent.

Related issues:

https://groups.google.com/g/subsurface-divelog/c/z02NK8FjFj4

Additional information:

test_summary.csv

Release note:

export: change format produced by 'CSV summary dive details' from TSV (tab separated) to CSV

Documentation change:

Mentions:

[bstoeger]

Well - congratulations on understanding that XSLT thing. I can't.

[mikeller]

Well - congratulations on understanding that XSLT thing. I can't.

Thanks. 😊
I think the hardest thing is to get your mind to stop looking at it as imperative list of steps - if you think of it as a 'more verbose form of regex' that doesn't care about sequence but works in terms of hierarchical matches and capture groups it's not too bad.
Unfortunately this isn't helped by the fact that in many cases authors try to hit it with a hammer long enough to make it work like an imperative programming language.

[mturkia]

@dirkhh Are our test cases run on pull requests nowadays? I fail to spot them from the checks list. There is a chance this change does not affect them, as it might be that the related tests are not testing against static file. But before this PR I was thinking that this was a one character change with quite a bit of tuning on our test cases.

[mturkia]

@mikeller My first guess is that the switch to tab separation was done before quoting the fields. Anyway, now that the fields are quoted, I do not see any reason to use tab as the separator.

Did you run the testcases? I do see that you did tweak the separator there, so maybe yes. I just want to make sure the tests still pass before approving, and am not able to run the tests myself right now.

[mikeller]

@mikeller My first guess is that the switch to tab separation was done before quoting the fields. Anyway, now that the fields are quoted, I do not see any reason to use tab as the separator.

👍

Quoting the fields (or at least the subset of fields potentially containing field separators, quotes, or line separators) and doubling all quotes inside the quoted fields is really all that is needed to generate CSV that follows the RFC and can be processed by pretty much everything that can process CSV.

Did you run the testcases? I do see that you did tweak the separator there, so maybe yes. I just want to make sure the tests still pass before approving, and am not able to run the tests myself right now.

I didn't initially, but then the pull request checks failed because I'd broken some tests. 😆 What I had to change was essentially switch the field separator from tab to comma when reading back the CSV output. So yes, the tests are run on pull requests.

[mturkia]

LGTM

[dirkhh]

@dirkhh Are our test cases run on pull requests nowadays? I fail to spot them from the checks list. There is a chance this change does not affect them, as it might be that the related tests are not testing against static file. But before this PR I was thinking that this was a one character change with quite a bit of tuning on our test cases.

Yes they do. At least they should