Skip to content

force strings with tabs to be wrapped in quotes#2

Closed
redchair123 wants to merge 1 commit intowarpech:masterfrom
redchair123:master
Closed

force strings with tabs to be wrapped in quotes#2
redchair123 wants to merge 1 commit intowarpech:masterfrom
redchair123:master

Conversation

@redchair123
Copy link

SheetClip.stringify does not wrap strings that contain tabs but no newlines.

To generate such a monstrosity, set

A1=1&CHAR(9)&2

On Excel 2011 it's displayed as if the tab is a single character, but copying and pasting (pbcopy/pbpaste) reveals that an actual tab character is there!

@warpech
Copy link
Owner

warpech commented Jan 8, 2013

I don't see tab character in your post above but maybe GitHub stripped it... Would you be able to writes steps to reproduce this error on Handsontable.com?

@redchair123
Copy link
Author

Here's one way to show the issue -- admittedly, there is also an issue in parse but this patch focuses just on the stringify part (this is on OSX with excel 2011, but I suspect the same issue exists on linux with libreoffice and windows with excel 2010/2013).

ISSUE IN PARSE

  1. copy the text "multi\ttab\ttest" to your clipboard:
$ echo -e "\"multi\ttab\ttest\"" | tee >(pbcopy)
"multi  tab test" <-- those are tabs
  1. paste in excel cell A1. On excel 2011 it displays as if the tabstop is 1: http://imgur.com/cpgj8

  2. On the grid at handsontable.com, just select (don't edit) and paste in B2: http://imgur.com/AKQ7C

It should dump in one cell but it now dumps in 3

ISSUE IN STRINGIFY

  1. copy the text "multi\ttab\ttest" to your clipboard:
$ echo -e "\"multi\ttab\ttest\"" | tee >(pbcopy)
"multi  tab test" <-- those are tabs
  1. On the grid at handsontable.com, edit cell A1 and paste: http://imgur.com/Lqyyb

  2. Copy from the grid and paste in excel: http://imgur.com/zGNj5

It should dump in 1 cell but in fact dumps in 3

Fixes

The attached commit fixes the stringify half. If you find the approach reasonable I'll also submit a stringify patch.

@redchair123 redchair123 closed this Apr 6, 2013
@warpech
Copy link
Owner

warpech commented Apr 8, 2013

Hi @Niggler, sorry that I didn't merge the PR so far. Now I see that you have deleted your fork but I am still able to see the changes that you've submitted. I agree this is a valid issue

@warpech
Copy link
Owner

warpech commented Apr 8, 2013

Did you also have a fix for the "parse" half of the issue?

@warpech warpech mentioned this pull request Apr 9, 2013
@redchair123
Copy link
Author

I've been investigating further and actually am not happy with the proposed solution. When I fully understand what's going on I'll submit another PR

@warpech
Copy link
Owner

warpech commented Apr 9, 2013

Thanks! Me too. I've spent whole last day unvestivating Excel and various CSV/TSV libraries to see if they perform better than my code. In places they are better, but still far from good. I've written more about it in issue #3. I would appreciate your help!

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.

3 participants

Comments