Bug 28363 - Always use semicolon when preference CSVDelimiter is missing
Summary: Always use semicolon when preference CSVDelimiter is missing
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 28327
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-17 10:41 UTC by Fridolin Somers
Modified: 2021-05-24 23:11 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28363: Always use semicolon when preference CSVDelimiter is missing (9.02 KB, patch)
2021-05-17 10:45 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2021-05-17 10:41:12 UTC
Some places use comma as value when preference CSVDelimiter is missing
or empty.
From installer/data/mysql/mandatory/sysprefs.sql whe see that defaut
install value is semicolon.

This patch sets semicolon when preference CSVDelimiter is missing.

"We are perl, we love semicolon ;)"

Test plan :
1) With SQL, set system preference 'CSVdelimiter' to empty string.
2) Create CSV export in impacted pages
3) Check columns are separated by semicolon character
Comment 1 Fridolin Somers 2021-05-17 10:45:57 UTC
Created attachment 121039 [details] [review]
Bug 28363: Always use semicolon when preference CSVDelimiter is missing

Some places use comma as value when preference CSVDelimiter is missing or empty.
From installer/data/mysql/mandatory/sysprefs.sql whe see that default install value is semicolon.

This patch sets semicolon when preference CSVDelimiter is missing.

"We are perl, we love semicolon ;)"

Test plan :
1) With SQL, set system preference 'CSVdelimiter' to empty string.
2) Create CSV export in impacted pages
3) Check columns are separated by semicolon character

https://bugs.koha-community.org/show_bug.cgi?id=28327
Comment 2 David Cook 2021-05-18 02:43:11 UTC
It seems strange to me to default to a semicolon. Shouldn't the default for Comma Separated Values be a comma?
Comment 3 Fridolin Somers 2021-05-18 08:19:36 UTC
In this case we change value in installer.

I propose a quick vote : comma or semicolon ?
Comment 4 Katrin Fischer 2021-05-22 16:55:50 UTC
Comma makes sense I guess - I believe this would be the case now mostly?
Comment 5 Fridolin Somers 2021-05-24 11:56:46 UTC
Ah HEA says :
https://hea.koha-community.org/systempreferences

delimiter says :
  - semicolon : 8925
  - comma 607

CSVDelimiter has only 19 comma.

But we can argue only a few install change default value.
Comment 6 Katrin Fischer 2021-05-24 12:08:56 UTC
(In reply to Fridolin Somers from comment #5)
> Ah HEA says :
> https://hea.koha-community.org/systempreferences
> 
> delimiter says :
>   - semicolon : 8925
>   - comma 607
> 
> CSVDelimiter has only 19 comma.
> 
> But we can argue only a few install change default value.

Rereading the bug I am happy with semicolon as it appears the most consistent choice.
Comment 7 David Cook 2021-05-24 23:11:53 UTC
(In reply to Fridolin Somers from comment #5)
> But we can argue only a few install change default value.

Exactly.

I don't know why we don't have a more normal default value...