stephen-hardy / xlsx.js

XLSX.js is a JavaScript library for converting the data in base64 XLSX files into JavaScript objects - and back! Please note that this library is licensed under the Microsoft Office Extensible File License - a license NOT approved by the OSI. While this license is based off of the MS-PL, which is OSI-approved, there are significant differences.
http://blog.innovatejs.com/?tag=xlsx-js
Other
575 stars 122 forks source link

jslint reports some errors. #15

Closed katoy closed 11 years ago

katoy commented 11 years ago

jslint rpoerts some errors for the xlsx.js (ver 2.3.0).

I fixed some errors. the Fixed xlsx.js passed "$ npm test"

$ diff xlsx.js xlsx.js-org 
1c1
< var JSZip = null;

---
> var JSZip = null
17c17
<   var result, zip = new JSZip(), zipTime, processTime, s, f, i, j, k, l, m, t, w, sharedStrings, styles, index, data, val, style, borders, border, borderIndex, fonts, font, fontIndex, edge,

---
>   var result, zip = new JSZip(), zipTime, processTime, s, f, i, j, k, l, t, w, sharedStrings, styles, index, data, val, style, borders, border, borderIndex, fonts, font, fontIndex,
19,20c19,20
<       numFmts = ['General', '0', '0.00', '#,##0', '#,##0.00',null,null,null,null, '0%', '0.00%', '0.00E+00', '# ?/?', '# ??/??', 'mm-dd-yy', 'd-mmm-yy', 'd-mmm', 'mmm-yy', 'h:mm AM/PM', 'h:mm:ss AM/PM',
<           'h:mm', 'h:mm:ss', 'm/d/yy h:mm',null,null,null,null,null,null,null,null,null,null,null,null,null,null, '#,##0 ;(#,##0)', '#,##0 ;[Red](#,##0)', '#,##0.00;(#,##0.00)', '#,##0.00;[Red](#,##0.00)',null,null,null,null, 'mm:ss', '[h]:mm:ss', 'mmss.0', '##0.0E+0', '@'],

---
>       numFmts = ['General', '0', '0.00', '#,##0', '#,##0.00',,,,, '0%', '0.00%', '0.00E+00', '# ?/?', '# ??/??', 'mm-dd-yy', 'd-mmm-yy', 'd-mmm', 'mmm-yy', 'h:mm AM/PM', 'h:mm:ss AM/PM',
>           'h:mm', 'h:mm:ss', 'm/d/yy h:mm',,,,,,,,,,,,,,, '#,##0 ;(#,##0)', '#,##0 ;[Red](#,##0)', '#,##0.00;(#,##0.00)', '#,##0.00;[Red](#,##0.00)',,,,, 'mm:ss', '[h]:mm:ss', 'mmss.0', '##0.0E+0', '@'],
52c52
<       // Load

---
>       // Load
175c175
<           merges = [];

---
>       merges = [];
196c196
<                       val = escapeXML(val);

---
>             val = escapeXML(val);
201c201
<                           index = sharedStrings[0].push(val) - 1; 

---
>                           index = sharedStrings[0].push(val) - 1; 
214c214
<                       val = null;

---
>                       val = null
217c217
<                       colWidth = String(val).length;

---
>                       colWidth = (''+val).length;
229c229
<                   if (!columns[j]) {

---
>                   if (columns[j] == null) {
233c233
<                       columns[j].autoWidth = true;

---
>                       columns[j].autoWidth = true
242,243c242,243
<                       merged = [j, 0];
<                       for (m = 0; m < cell.colSpan-1; m++) {

---
>                       merged = [j, 0]
>                       for (var m = 0; m < cell.colSpan-1; m++) {
250c250
<                       for (m = 1; m < cell.rowSpan; m++) {

---
>                       for (var m = 1; m < cell.rowSpan; m++) {
252c252
<                               data[i+m].splice(j, 0, cell);

---
>                               data[i+m].splice(j, 0, cell)
268c268
<                   if (val !== null) {

---
>                   if (val != null) {
277c277
<           cols = [];

---
>           cols = []
341c341
<               style.formatCode = index;

---
>               style.formatCode = index
343c343
<               style.formatCode = 0;

---
>               style.formatCode = 0
347c347
<           borderIndex = 0;

---
>           borderIndex = 0
349c349
<               border = ['<border>'];

---
>               border = ['<border>']
351c351
<               for (edge in {left:0, right:0, top:0, bottom:0, diagonal:0}) {

---
>               for (var edge in {left:0, right:0, top:0, bottom:0, diagonal:0}) {
373c373
<           fontIndex = 0;

---
>           fontIndex = 0
375c375
<               font = ['<font>'];

---
>               font = ['<font>']

But the fixed xlsx.js has one error. I have no idea how to fix the error.

jslint xlsx.js
$ jslint xlsx.js
Error at line 352 character 2: The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
for (edge in {left:0, right:0, top:0, bottom:0, diagonal:0}) {
dduponchel commented 11 years ago

The last error comes from javascript which can do funny things if you augment Object.prototype. See http://yuiblog.com/blog/2006/09/26/for-in-intrigue/

stephen-hardy commented 11 years ago

Hello Katoy,

Thanks for the contribution. Since this is not in the form of a pull request, I'm going to close this issue and take a note to run everything through jsLint when I do a review of the recent contributions.

I'll probably be looking to change the iteration over the object literal's properties, as that will not be very performant. However, I haven't had a chance to examine the context yet.

Stephen