fix: missing PRN in xlsx.mini #1908

2020-04-20T07:02:41Z

chenxeed commented

2020-04-20 07:02:41 +00:00

(Migrated from github.com)

Issue

https://github.com/SheetJS/sheetjs/issues/1760

Description

Based on my investigation, it seems like the function read_prn in the mini is missing PRN when it tries to read a string of CSV file.

My proposed solution is to add the PRN declaration as part of the mini build, which exists in the bits/40_harb.js.

# Issue https://github.com/SheetJS/sheetjs/issues/1760 # Description Based on my investigation, it seems like the function `read_prn` in the mini is missing `PRN` when it tries to read a string of CSV file. My proposed solution is to add the `PRN` declaration as part of the `mini` build, which exists in the `bits/40_harb.js`.

chenxeed commented

2020-04-20 07:45:10 +00:00

(Migrated from github.com)

@SheetJSDev I have no written unit test in this changes, but I have tested the changes in my local application and the CSV works. Also, the existing unit test seems to pass when I run the test locally.

@SheetJSDev I have no written unit test in this changes, but I have tested the changes in my local application and the CSV works. Also, the existing unit test seems to pass when I run the test locally. ![test import svg](https://user-images.githubusercontent.com/3530355/79726711-ce7fe580-831d-11ea-8dc7-ac689f35907f.gif)

chenxeed commented

2020-05-20 07:12:04 +00:00

(Migrated from github.com)

Poke @SheetJSDev again 🙇 it would be great to have this settled for the next release, so we can use xlsx.mini in our application ☕

Poke @SheetJSDev again 🙇 it would be great to have this settled for the next release, so we can use `xlsx.mini` in our application ☕

chenxeed commented

2020-06-16 07:21:07 +00:00

(Migrated from github.com)

Hi @SheetJSDev @srijonsaha may I have an update or response for the PR?

If there's anything missing from the PR, please let me know to improve.

I'm waiting for this patch before I can use xlsx.mini in my production project.

Thanks!

Hi @SheetJSDev @srijonsaha may I have an update or response for the PR? If there's anything missing from the PR, please let me know to improve. I'm waiting for this patch before I can use `xlsx.mini` in my production project. Thanks!

SheetJSDev commented

2020-06-16 07:24:09 +00:00

(Migrated from github.com)

@garrettluu can you take a peek?

❤️ 1

garrettluu (Migrated from github.com) requested changes 2020-06-16 18:48:09 +00:00

garrettluu (Migrated from github.com) left a comment

Code itself looks fine, but please squash your commits into 1 with git rebase, since we don't want too many commits to clutter the history.

Code itself looks fine, but please squash your commits into 1 with `git rebase`, since we don't want too many commits to clutter the history.

chenxeed commented

2020-06-18 05:53:41 +00:00

(Migrated from github.com)