search

bnvk / Conjuror

An experiment in the CSV format, open data, magic, and wizardly things!
Other
11 stars 2 forks source link

Fix that borked PR #25

Closed simonv3 closed 9 years ago

simonv3 commented 9 years ago

And now with tests for dates!

One of the tests fails because I'm not sure whether when you pass "Apr" as a trim, it should just return stuff for this April, rather than all Aprils.

Thoughts @bnvk?

bnvk commented 9 years ago

@simonv3 aced! Well done compadre ;)

simonv3 commented 9 years ago

@bnvk What are your thoughts on the failing test? Should a search for "Apr" return all Aprils? Or only Aprils of this year?

bnvk commented 9 years ago

@simonv3 thanks for asking for clarification. My initial instinct re: this is that we pass a second word like -d april all or parse a hyphen like -d april-all what do you think?

simonv3 commented 9 years ago

Agreed.

I think my expectation is (would be as an outsider too) that "April" would find the most recent April. However! It would have to be the most recent April in the past I think. So say "Jun" today would show me June of 2014, rather than doing June of this year - 2015.

bnvk commented 9 years ago

Right. Exactly. That's actually an interesting case (dates in past year) I'm not sure how the code handles at the moment. I actually wonder if we should investigate using another node library at this point or if moment.js will be sufficient

simonv3 commented 9 years ago

I don't actually know any other date libraries. I can picture how to do this, and it wouldn't be overly complex in moment.