Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date support #19

Open
symbyte opened this issue Mar 30, 2017 · 19 comments
Open

Date support #19

symbyte opened this issue Mar 30, 2017 · 19 comments

Comments

@symbyte
Copy link

symbyte commented Mar 30, 2017

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
Copy link
Contributor

mhkeller commented Oct 4, 2017

@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
Copy link

@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
Copy link

@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
Copy link

@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
Copy link
Contributor

mhkeller commented Oct 26, 2017

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
Copy link

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
Copy link
Contributor

tmcw commented Oct 27, 2017

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

@mhkeller
Copy link
Contributor

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

@tmcw
Copy link
Contributor

tmcw commented Oct 31, 2017

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
Copy link
Contributor

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
Copy link

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

@davidtheclark
Copy link
Contributor

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
Copy link
Contributor

mhkeller commented Nov 8, 2017

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

@davidtheclark
Copy link
Contributor

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
Copy link

@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
Copy link
Contributor

@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
Copy link

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

@sheindel
Copy link
Contributor

sheindel commented Feb 8, 2023

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants