mapbox / dbf

dbf writer
BSD 2-Clause "Simplified" License
37 stars 56 forks source link

Date support #19

Open symbyte opened 7 years ago

symbyte commented 7 years ago

Would it be possible to have javascript date support? From the source of structure.js it looks like it is at least partially supported, but is there anything blocking having the fields.js script resolve date types in addition to numbers, strings, and booleans?

Willing to submit a PR, but didn't want to waste my time if it wasn't possible/desired.

mhkeller commented 7 years ago

@symbyte I have a fork at https://github.com/mhkeller/dbf/ that includes some of the unmerged PRs for this. If you wanted to add date support to that fork that would be great.

bakednevada commented 7 years ago

@symbyte Column resolution is based on some typeof magic, but you can't detect dates that way. Dates have to be resolved with instanceof. Took a grand total of about 5 minutes to patch it in:

diff --git a/json2dbf.js b/json2dbf.js
index 758d2a5..a0ed2e4 100644
--- a/json2dbf.js
+++ b/json2dbf.js
@@ -2,7 +2,7 @@ var dbf = require('./'),
     fs = require('fs');

 var buf = dbf.structure([
-    {foo:'bar',noo:10},
+    {foo:'bar',noo:new Date()},
     {foo:'louie'}
 ]);

diff --git a/src/fields.js b/src/fields.js
index 5961b76..27c9285 100644
--- a/src/fields.js
+++ b/src/fields.js
@@ -4,6 +4,7 @@ var types = {
     string: 'C',
     number: 'N',
     boolean: 'L',
+    date: 'D',
     // type to use if all values of a field are null
     null: 'C'
 };
@@ -36,7 +37,7 @@ function inherit(a, b) {

 function obj(_) {
     var fields = {}, o = [];
-    for (var p in _) fields[p] = _[p] === null ? 'null' : typeof _[p];
+    for (var p in _) fields[p] = _[p] === null ? 'null' : _[p] instanceof Date ? 'date' : typeof _[p];
     for (var n in fields) {
         var t = types[fields[n]];
         if(t){
diff --git a/src/lib.js b/src/lib.js
index 0c7ff80..5b60ed0 100644
--- a/src/lib.js
+++ b/src/lib.js
@@ -31,3 +31,8 @@ module.exports.writeField = function writeField(view, fieldLength, str, offset)
     }
     return offset;
 };
+
+module.exports.writeDate = function(date) {
+    if(!date || isNaN(date.getTime())) return "        ";
+    return ("0000"+date.getFullYear()).slice(-4) + ("00"+(date.getMonth()+1)).slice(-2) + ("00"+date.getDate()).slice(-2);
+};
diff --git a/src/structure.js b/src/structure.js
index 4b02783..9bc286d 100644
--- a/src/structure.js
+++ b/src/structure.js
@@ -29,7 +29,7 @@ module.exports = function structure(data, meta) {
     view.setUint8(0, 0x03);
     // date of last update
     view.setUint8(1, now.getFullYear() - 1900);
-    view.setUint8(2, now.getMonth());
+    view.setUint8(2, now.getMonth()+1);
     view.setUint8(3, now.getDate());
     // number of records
     view.setUint32(4, data.length, true);
@@ -75,7 +75,7 @@ module.exports = function structure(data, meta) {
                 // date
                 case 'D':
                     offset = lib.writeField(view, 8,
-                        lib.lpad(val.toString(), 8, ' '), offset);
+                        lib.writeDate(val), offset);
                     break;

                 // number

Checked against the DBF reader from https://www.npmjs.com/package/xlsx and it appears to work correctly.

jeff-tenhave commented 7 years ago

@bakednevada Are you planning on submitting a PR with that change? I'd like to avoid forking if possible so it would be great if we could get that change in here. I could submit a PR if you don't plan to.

bakednevada commented 7 years ago

@jeff-tenhave go ahead, but I recommend also submitting a PR to the @mhkeller branch, as his branch seems to be ahead of the mainline

mhkeller commented 7 years ago

It would be great if this were merged into here as opposed to a fork. There are a few outstanding PRs that would also be great. I made a fork because it seemed this repo hasn't been maintained in the last six months and I needed a fix for another lib that depends on this.

@tmcw I don't think you're at Mapbox anymore. Is there someone you could alert there who might be able to take this one over?

bakednevada commented 7 years ago

If the name is lost, I wouldn't mind occasionally helping on a fork. We could organize with a github org like dbfjs and an npm module dbfjs since that name is available on github and npm. And if we are able to obtain access to publishing under the name, we could have dbf merely wrap dbfjs.

tmcw commented 7 years ago

I'm not sure. I can probably poke @davidtheclark without him getting mad at me :)

mhkeller commented 7 years ago

Tom you have given the world so much, how could one ever be mad ;)

tmcw commented 7 years ago

Well, next up, poking @mikelmaron - anyone over there want to merge some pull requests? There are many others, all waiting for that green button.

davidtheclark commented 7 years ago

Hi @tmcw and everyone. I'm asking around to try to sort out whether anybody internal wants to take on this project. If I can't figure out anything definite this week, I'll learn what's going on here and merge some PRs next week. Hope that works for everyone.

jeff-tenhave commented 7 years ago

I've opened a PR with @bakednevada 's changes.

davidtheclark commented 7 years ago

I think the best thing moving forward would be for folks to switch over to @mhkeller's fork. We would then add a note to the readme of this repo recommending that people go use that fork. Once the fork is released on npm, we can deprecate the dbf package and point users to the new one.

@mhkeller: Does that sound ok with you? Interested in taking over maintenance? If you're not and anybody else in this thread is, please let me know.

cc @mikelmaron @kathleenlu09

mhkeller commented 7 years ago

@davidtheclark i unfortunately wouldn't have the expertise to maintain this one.

davidtheclark commented 7 years ago

Ok, @mhkeller.

I added a note to the README saying that this project is looking for a new maintainer: https://github.com/mapbox/dbf/blob/master/README.md#looking-for-new-maintainers. I plan to sit on it a bit to see if anyone bites before taking any more steps.

bakednevada commented 6 years ago

@davidtheclark in absence of a proper maintainer, if you're willing to spend a few minutes there's some low-hanging fruit. #20 and #24 are trivially mergeable, #24 would also close issue #23.

davidtheclark commented 6 years ago

@bakednevada happy to add you as a collaborator on GitHub and npm if you'd like to carry this out. Does that sound ok? Please let me know your npm username and I can do that today.

bakednevada commented 6 years ago

I'm "bakednevada" on npm: https://www.npmjs.com/~bakednevada , no rush I'll look into it tonight

sheindel commented 1 year ago

Hi All! Since this issue had a lot of people interested in this repo, I just wanted to flag that I've taken over maintenance. I'm hoping to clear out some of these older issues and do some much needed updates. I'll try to review the related PR to see if we can get this merged.

mtrsklnkvMM commented 1 year ago

@sheindel hey, any updates for the Date support PR ? Thanks