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

Can't get date ranges to render #58

Open
jrochkind opened this Issue Mar 7, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@jrochkind
Contributor

jrochkind commented Mar 7, 2018

I am having trouble getting date ranges to render.

I am not sure if I am doing something wrong, it is a bug in my input, or it is a bug in the CSL style (or perhaps the CSL style I am using refuses to do it?). But looking at the CSL docs and playing with custom CSL styles... I don't think it's the CSL style.

I don't see any examples of date ranges in spec/citeproc/ruby/renderer/date_spec.rb. Or can't find any other examples in specs to use as a guide.

Is it possible date ranges don't work? Or aren't being properly imported from CSL data input json?

Can you provide any advice, or a working example of something rendering a date range, like "1900-1910" when that's in the citation?

@inukshuk

This comment has been minimized.

Owner

inukshuk commented Mar 8, 2018

Here is a test case that currently fails and judging by this TODO comment it is indeed missing. Sorry! I'll try to take a stab at it over the weekend.

@jrochkind

This comment has been minimized.

Contributor

jrochkind commented Mar 8, 2018

Oh, okay, glad to have an explanation!

I guess travis isn't running the cucumber features, only the rspec? Is why the test case can fail with travis showing green?

@jrochkind

This comment has been minimized.

Contributor

jrochkind commented Mar 8, 2018

Also I don't neccesarily expect you to fix this right away, although I'd of course be quite happy if you did!

Alternatives are listing it in the README to note that it doesn't work, and giving me some advice/guidance in where I find a spec (or automated tests from a non-ruby project?) for how it's supposed to behave and I'll see if I can get to it.

@inukshuk

This comment has been minimized.

Owner

inukshuk commented Mar 8, 2018

This is the relevant section in the spec; in the folder of the test case I linked to above there are many other tests (just grep for 'range' in that folder, e.g. this one) -- they are in the citeproc repo because I used to run them against citeproc-ruby and -js (via Ruby); one of the main reasons for the repo split. These tests are from citeproc-js and many of them test specific citeproc-js features that's why they are not run on Travis CI.

@jrochkind

This comment has been minimized.

Contributor

jrochkind commented Mar 12, 2018

Hey @inukshuk , I suspect you didn't have time to get to this this weekend -- I'll try to get a PR this week, or else next. Let me know if you're already working on it though, so I don't duplicate what you are already working on! Otherwise I'll try to get it in!

@jrochkind

This comment has been minimized.

Contributor

jrochkind commented Mar 13, 2018

Woah, getting date ranges to work within the current architecture is harder than I expected, heh. Still working on it, but any advice for how you might go about it appreciated.

@inukshuk inukshuk referenced a pull request that will close this issue Mar 13, 2018

Open

Render date ranges #59

@inukshuk

This comment has been minimized.

Owner

inukshuk commented Mar 13, 2018

Yeah, I got stuck over the weekend, because I fell into a rabbit hole (this usually happens with CSL): I'm not sure how season ranges are supposed to be rendered (since the season is set for the whole date not the part), but I might be missing something.

I pushed #59 which is as far as I got. I added three line comments -- most notably, something along the lines of date.delimit_range_at should be added to CiteProc::Date, returning the name of the first part (day, month, year, in that order, not the order of the date-part notes) that is different. This also doesn't address 'open' ranges yet (which the spec does not mention I believe?).

@inukshuk

This comment has been minimized.

Owner

inukshuk commented Mar 14, 2018

OK, cleared up the season issue. Seems like citeproc-js interprets month-part 13-16 as seasons. In addition, it would make sense to accept 21-24 there as well (the numbers used by ISO 8601 for seasons, independent of hemisphere).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment