fix: missing PRN in xlsx.mini #1908
No reviewers
Labels
No Label
DBF
Dates
Defined Names
Features
Formula
HTML
Images
Infrastructure
Integration
International
ODS
Operations
Performance
PivotTables
Pro
Protection
Read Bug
SSF
SYLK
Style
Write Bug
good first issue
No Milestone
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sheetjs/sheetjs#1908
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "chx/fix-mini-prn"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Issue
https://github.com/SheetJS/sheetjs/issues/1760
Description
Based on my investigation, it seems like the function
read_prn
in the mini is missingPRN
when it tries to read a string of CSV file.My proposed solution is to add the
PRN
declaration as part of themini
build, which exists in thebits/40_harb.js
.@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.
Poke @SheetJSDev again 🙇 it would be great to have this settled for the next release, so we can use
xlsx.mini
in our application ☕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!
@garrettluu can you take a peek?
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.Thanks @garrettluu for the response and review. I've squashed the commit. Look forward to the update 👍
Hi @garrettluu @SheetJSDev I've made the requested changes. Please confirm if this is fine. Look forward to the release! 🙇