Ignoring empty cells

[GoogleCodeExporter]

Found in most recent versions of Chrome and Firefox.

To reproduce:  $.csvEntry2Array("1,2,,4"), which results in ["1", "2", "4"], 
when ideally we want ["1", "2", "", "4"].

I wrote a patch that works well for me. Here's the patch, but maybe the 
developer would like to reinclude this as an option, there might be someone who 
wants empty cells discarded.

--- a/inc/jquery.csv-0.61.js
+++ b/inc/jquery.csv-0.61.js
@@ -92,6 +92,8 @@ RegExp.escape= function(s) {
         output.push(m1.replace(reDelimiterUnescape, delimiter));
       } else if(typeof m2 === 'string' && m2.length) { // Fix: eval
         output.push(m2);
+      } else if(typeof m0 === 'string' && typeof m1 === 'undefined'
+        output.push("");
       }
       return '';
     });

Original issue reported on code.google.com by Ryan.Txa...@gmail.com on 4 Sep 2012 at 11:40

[GoogleCodeExporter]

Oops, my copy/paste missed the scroll. Here is the correct paste: 

--- a/inc/jquery.csv-0.61.js
+++ b/inc/jquery.csv-0.61.js
@@ -92,6 +92,8 @@ RegExp.escape= function(s) {
         output.push(m1.replace(reDelimiterUnescape, delimiter));
       } else if(typeof m2 === 'string' && m2.length) { // Fix: evaluates to false for both empty strings and undefined
         output.push(m2);
+      } else if(typeof m0 === 'string' && typeof m1 === 'undefined' && m0 === 
separator) {
+        output.push("");
       }
       return '';
     });

Original comment by Ryan.Txa...@gmail.com on 4 Sep 2012 at 11:41

[GoogleCodeExporter]

@Ryan.Txanson Thank you for the fix. If you would attach the actual .patch file 
that will make life a little easier.

As soon as the patch is applied and the remote origin is updated, I will post 
an update here. 

Original comment by evanpla...@gmail.com on 5 Sep 2012 at 7:09

[GoogleCodeExporter]

This conflicts with the fix I submitted. If anything, I think we're both being 
too conservative, and that else if { ... } should actually just be else {...}

There's simply no case where it should fail to output a value. That's clear 
from both standard and usage.

Original comment by r...@acm.org on 5 Sep 2012 at 10:47

[GoogleCodeExporter]

(I'm referring to the patches I submitted with #5).

Original comment by r...@acm.org on 5 Sep 2012 at 10:48

[GoogleCodeExporter]

@Ryan.Txanson I'm pretty sure this issue was fixed in bug 5.

I'm not exactly sure why but your fix actually re-breaks it. You can pull the 
latest from the repository to verify whether or not there's still an issue. 
Otherwise, if there's no existing issue I'd like to close this out and move on.

Original comment by evanpla...@gmail.com on 5 Sep 2012 at 5:58

[GoogleCodeExporter]

Ah, indeed. I just tested with the latest revision checked out from git, and 
the problem is not there, everything works in Chrome and Firefox. I didn't 
think to actually check out the source, and instead was using 0.61, the latest 
stable revision, which was the one I applied the fix to. You can close the 
issue. :)

Original comment by Ryan.Txa...@gmail.com on 5 Sep 2012 at 9:05

[GoogleCodeExporter]

@Ryan.Txanson The code was actually just fixed last night. The other patch 
fixed this issue as a side effect. If it hadn't, your code would still be 
relevant.

Thanks anyway for your help. Every little bit that people contribute helps to 
make this a more robust and versatile library.

Original comment by evanpla...@gmail.com on 5 Sep 2012 at 9:10

[GoogleCodeExporter]

It helps to have a good test runner. Otherwise I'd probably be applying your 
patch to code that isn't broken. ;)

Original comment by evanpla...@gmail.com on 5 Sep 2012 at 9:12

[GoogleCodeExporter]

Original comment by evanpla...@gmail.com on 5 Sep 2012 at 9:12

• Changed state: Verified

[GoogleCodeExporter]

Haha, yes. I just put the last git revision into production as well, and 
everything works as before. So, hurray!

Original comment by Ryan.Txa...@gmail.com on 5 Sep 2012 at 9:14

[GoogleCodeExporter]

Original comment by evanpla...@gmail.com on 11 Oct 2012 at 4:07