257767 - Option to have CSV files separated with semicolons instead of commas

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Marc Onteniente]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0) Opera 7.23  [fr]
Build Identifier: 

Just an Idea:
I maodified it for me: when I click on CSV, in a report, I got excel open with 
every thing welle formated, because of my ";" instead of ","

Marc O.

Reproducible: Always
Steps to Reproduce:
1. CSV in each report
2. open file ".CSV" in excel


Actual Results:  
fields are melt in on cell.

Expected Results:  
have ONE number by cell

Comment 1

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 2

[Marc Schumann [:Wurblzap]]

Attachment: Patch

Now that we have settings, we can do this.

Comment 3

[Marc Schumann [:Wurblzap]]

This fixes a regression of bug 296180 along the way (not separating status and
resolution texts).

Comment 4

[Marc Schumann [:Wurblzap]]

Attachment: Patch 2

Comment 5

[Marc Schumann [:Wurblzap]]

Blocking a blocker (bug 299801), so this should be a blocker.

Comment 6

[Max Kanat-Alexander]

Yep, Marc has a point. Blocker. :-)

Comment 7

[Frédéric Buclin]

Comment on attachment 188414 [details] [diff] [review]
Patch 2

>Index: Bugzilla/User/Setting.pm

>+sub validate_value {

Missing POD docs for this new method.



>Index: template/en/default/list/list.csv.tmpl

>+bug_id[% colsepchar %]
> [% FOREACH column = displaycolumns %]
>+  [% column FILTER csv %][% colsepchar %]
> [% END %]
> 
> [% FOREACH bug = bugs %]
>+  [% bug.bug_id %][% colsepchar %]
>     [% FOREACH column = displaycolumns %]
>       [% IF column == "opendate" OR column == "changeddate" %]
>         [% rawcolumn = column.replace("date", "time") %]
>         [% bug.$column = date.format(bug.$rawcolumn, "%Y-%m-%d %H:%M:%S") %]
>       [% END %]
>+      [% bug.$column FILTER csv %][% colsepchar %]
>     [% END %]
> 
> [% END %]

Nit: as discussed on IRC, removing [% colsepchar %] after 'bug_id' and [%
bug.bug_id %], and moving the ones in the FOREACH loop *before* the data [%
<data> FILTER csv %] would prevent the trailing comma or semicolon at the end
of lines. I did it while testing your patch and it works fine.


>Index: template/en/default/reports/report-table.csv.tmpl

>+[% title %][% colsepchar %]
> [% IF col_field -%]
> [% FOREACH col = col_names -%]
>   [% IF col_field == 'bug_status' %]
>@@ -48,11 +50,11 @@
>   [% ELSIF col_field == 'resolution' %]
>     [% resolution_descs.$col FILTER csv -%]
>   [% ELSE %]
>+    [% col FILTER csv -%]
>+  [% END %][% colsepchar %]
> [% END -%]
> [% ELSE -%]
>+  [% num_bugs %][% colsepchar %]
> [% END %]
> 
> [% FOREACH row = row_names %]
>@@ -61,14 +63,14 @@
>   [% ELSIF row_field == 'resolution' %]
>     [% resolution_descs.$row FILTER csv -%]
>   [% ELSE %]
>+    [% row FILTER csv -%]
>+  [% END %][% colsepchar %]
>   [% FOREACH col = col_names %]
>     [% IF data.$tbl AND data.$tbl.$col AND data.$tbl.$col.$row %]
>+      [% data.$tbl.$col.$row -%]
>     [% ELSE %]
>+      [% -%]0
>+    [% END %][% colsepchar %]
>   [% END %]
> 
> [% END %]

Nit: while you are changing this file, could you please fix the indentation
too? I had to did it manually in order to review your patch. Also, by moving [%
colsepchar %] at the head of the FOREACH loop avoid us to have these trailing
commas or semicolons again.


If you don't want to fix these nits in this patch, please open a bug and CC me
on it. I tested your patch on both MySQL and PostgreSQL and also used
OpenOffice 2.0 to view the CSV files and it works great! So you add at least
the POD docs for validate_value() in Setting.pm and you have my r+.

Comment 8

[Marc Schumann [:Wurblzap]]

Attachment: Patch 2.1

Added POD.
Nicer (non-trailing) [% colsepchar %]ing.
Indentation fixed.
Bonus fix: got rid of the needless [% title %] in report-table.csv.tmpl.

Comment 9

[Frédéric Buclin]

Comment on attachment 189743 [details] [diff] [review]
Patch 2.1

> [% setting_descs = { 

>+   "csv_colsepchar"                   => "Column separator character for CSV downloads",

Nit: what about: "Field separator character for CSV files"? If you like it, I
could change it on checkin.


>Index: template/en/default/reports/report-table.csv.tmpl

>+  [% FOREACH col = col_names -%]
>+  [% colsepchar %]
>+    [% IF col_field == 'bug_status' %]

Nit: [% colsepchar %] should be aligned with [% IF %]. I will fix that on
checkin.


Tested with both "," and ";". Works great! r=LpSolit

Comment 10

[Marc Schumann [:Wurblzap]]

Attachment: Patch 2.2

Nits of comment 9 addressed.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

If semicolons are better separators, we should use them consistently, not give
users an option to change to them.

I'm not averse to many rarely used preferences; on the contrary, most
preferences will be rarely used.  But nothing in this bug has given any
indication of good reasons to give users this option.

The reporter certainly didn't, saying only that semicolons worked when commas
didn't.  That doesn't mean we should let users pick between them (otherwise
we're making the user fix our bugs), it means we should fix the CSV format so
that it works for users, whether that means switching to semicolons,
double-quoting comma-containing values, or by some other means.

We need to fix the problem here, not shunt it off to a user preference.

Comment 12

[Marc Schumann [:Wurblzap]]

(In reply to comment #11)

Neither nor is a better separator. Either field separator may be the wrong one,
or the right one.
By definition, commas should separate fields in CSV files. But Windows uses a
(language-setting dependent) "List separator" character as CSV field separator.
Known characters used as "List separator" are comma (English language settings)
and semicolon (German language settings).

By introducing a user preference, users can make Bugzilla work with their
personal language setting.

Myk, please reconsider.

Comment 13

[Frédéric Buclin]

I'm with Marc.

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Ah, I see.  Ok, then it makes sense to let users set this preference as they set
it (or as it gets set for them) in their operating system.

Comment 15

[Frédéric Buclin]

tip:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.418; previous revision: 1.417
done
Checking in editsettings.cgi;
/cvsroot/mozilla/webtools/bugzilla/editsettings.cgi,v  <--  editsettings.cgi
new revision: 1.3; previous revision: 1.2
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.77; previous revision: 1.76
done
Checking in Bugzilla/User/Setting.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User/Setting.pm,v  <--  Setting.pm
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v 
<--  filterexceptions.pl
new revision: 1.45; previous revision: 1.44
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.55; previous revision: 1.54
done
Checking in template/en/default/global/setting-descs.none.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/setting-descs.none.tmpl,v
 <--  setting-descs.none.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/list/list.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.csv.tmpl,v  <--
 list.csv.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/reports/chart.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.csv.tmpl,v
 <--  chart.csv.tmpl
new revision: 1.2; previous revision: 1.1
done
Checking in template/en/default/reports/report-table.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-table.csv.tmpl,v
 <--  report-table.csv.tmpl
new revision: 1.7; previous revision: 1.6
done


2.20rc1:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.412.2.3; previous revision: 1.412.2.2
done
Checking in editsettings.cgi;
/cvsroot/mozilla/webtools/bugzilla/editsettings.cgi,v  <--  editsettings.cgi
new revision: 1.2.4.1; previous revision: 1.2
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.75.4.2; previous revision: 1.75.4.1
done
Checking in Bugzilla/User/Setting.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User/Setting.pm,v  <--  Setting.pm
new revision: 1.4.2.1; previous revision: 1.4
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v 
<--  filterexceptions.pl
new revision: 1.43.2.1; previous revision: 1.43
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.52.2.1; previous revision: 1.52
done
Checking in template/en/default/global/setting-descs.none.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/setting-descs.none.tmpl,v
 <--  setting-descs.none.tmpl
new revision: 1.3.4.1; previous revision: 1.3
done
Checking in template/en/default/list/list.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.csv.tmpl,v  <--
 list.csv.tmpl
new revision: 1.4.4.1; previous revision: 1.4
done
Checking in template/en/default/reports/chart.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.csv.tmpl,v
 <--  chart.csv.tmpl
new revision: 1.1.10.1; previous revision: 1.1
done
Checking in template/en/default/reports/report-table.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-table.csv.tmpl,v
 <--  report-table.csv.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done

Comment 16

[Marc Schumann [:Wurblzap]]

Thanks all!