adding an option to not parse numbers #2014

2020-06-18T19:25:51Z

ab320012 commented

2020-06-18 19:25:51 +00:00

(Migrated from github.com)

i have a problem that this PR will solve let me know if it needs anything else to be included.

srijonsaha (Migrated from github.com) reviewed 2020-06-18 19:25:51 +00:00

SheetJSDev commented

2020-06-18 19:36:59 +00:00

(Migrated from github.com)

Let's take a step back.

What you changed affects the CSV writer, not the CSV parser. Is that the intention?
Is the goal specifically to write raw numbers or should it also write raw strings / boolean / dates as well?
If the goal is to change the CSV writing, the changes should only be in make_csv_row. I would have written something like:

-			txt = ''+format_cell(val, null, o);
+			txt = ''+(o.rawNumbers && val.t == "n" ? val.v : format_cell(val, null, o));

Let's take a step back. 1) What you changed affects the CSV writer, not the CSV parser. Is that the intention? 2) Is the goal specifically to write raw numbers or should it also write raw strings / boolean / dates as well? 3) If the goal is to change the CSV writing, the changes should only be in `make_csv_row`. I would have written something like: ```diff - txt = ''+format_cell(val, null, o); + txt = ''+(o.rawNumbers && val.t == "n" ? val.v : format_cell(val, null, o)); ```

ab320012 commented

2020-06-18 20:46:13 +00:00

(Migrated from github.com)

i just need to be able to write the numbers raw but keep the formatting the same otherwise i cna make the requested change

ab320012 commented

2020-06-18 20:46:31 +00:00

(Migrated from github.com)

i'll add it to sheet_to_json as well just in case?

SheetJSDev commented

2020-06-18 20:54:44 +00:00

(Migrated from github.com)

sheet_to_json already has the option raw. When true (default) it will give the underlying values, and when false (have to explicitly pass the option) it will use the formatted text. The difference between raw and your proposed rawNumbers is that the former works for all types and the latter is only for numbers (hence question 2)

`sheet_to_json` already has the option `raw`. When true (default) it will give the underlying values, and when false (have to explicitly pass the option) it will use the formatted text. The difference between `raw` and your proposed `rawNumbers` is that the former works for all types and the latter is only for numbers (hence question 2)

ab320012 commented

2020-06-18 21:35:42 +00:00

(Migrated from github.com)

Hey yes it's just to write raw numbers but keep the other formatting, I will make the change you asked

ab320012 commented

2020-06-19 15:49:21 +00:00

(Migrated from github.com)

@srijonsaha great library, not having this is causing some production issues with our website wondering if you can approve

ab320012 commented

2020-07-03 16:17:29 +00:00

(Migrated from github.com)

@SheetJSDev @srijonsaha hey just bumping this i still need it for my prod application

srijonsaha commented

2020-07-06 22:55:22 +00:00

(Migrated from github.com)

@ab320012 Hi could you give a common example use case of what this change would be? We don't want to accept every change simply because it solves a very specific one time situation.

ab320012 commented

2020-07-08 03:03:57 +00:00

(Migrated from github.com)

@srijonsaha yes absolutely, the number parser right now is formatting my csv in scientific notation, i'd like to keep the numbers raw but parse everything else the same way that it was, is that enough of a use case? it's just an added option so i can use the internal parser but keep the numbers the way they are does that make sense?

👍 1

ab320012 commented

2020-07-10 16:28:58 +00:00

(Migrated from github.com)

@srijonsaha hey thanks for the merge this is causing us some prod issues, wondering if you will publish soon?

Sign in to join this conversation.

No reviewers

srijonsaha