-
Notifications
You must be signed in to change notification settings - Fork 98
feat: Add Book for bibliography
#443
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
base: main
Are you sure you want to change the base?
Conversation
| let authors ← andList <$> p.authors.mapM go | ||
| return {{ | ||
| {{authors}} s!", {p.year}. " {{ link {{"“" {{← go p.title}} "”"}} }} ". " | ||
| {{ p.series.map (λ (s, v) => {{ "Volume " <strong>s!"{v}"</strong> " of " /- TODO: insert series -/ ", "}}) |>.getD .empty }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it's rare for a book reference to include only series or volume without the other, so I’ve treated them as a pair. However, I’m unsure how to represent this pairing properly in the citation formatting (manupulating Html or etc. is too complex).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment on datatype
| isbn : Option String := none -- TODO: make concrete type `ISBN10` or `ISBN13` | ||
| publisher : Doc.Inline Manual | ||
| edition : Option (Doc.Inline Manual) := none | ||
| series : Option ((Doc.Inline Manual) × Int) := none |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this misunderstands "volume" in citations.
I would use volume for a single work that's published in multiple physical books, e.g. The Art of Computer Programming by Knuth. A series is usually a collection of disparate books by multiple authors created over a period of time, and it's IMHO less important here.
That is, if I'm citing TAoCP, it matters a lot whether it's vol. 1 or 3. But knowing that a book is part of LNCS is less important.
What about:
| series : Option ((Doc.Inline Manual) × Int) := none | |
| /-- Volume in a multi-volume work -/ | |
| volume : Option Nat | |
| /-- Series name and position within it, e.g. "Great Lectures in Mathematics 47" | |
| series : Option ((Doc.Inline Manual) × Int) := none |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have great opinion, so if it is more suitable (like bibtex), I'll change.
| | thesis : Thesis → Citable | ||
| | arXiv : ArXiv → Citable | ||
| | article : Article → Citable | ||
| | book : Book → Citable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, but I do feel like this datatype is getting out of hand and it will soon be time to find an open representation here rather than the current one. A type class, perhaps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this is not something for you to do, unless you're inspired)
|
Thanks! |
|
Sorry for lating to reply for reviewing. I'll fix |
|
I can't spare time to fix this issue so I closed this PR. |
a641b1c to
990507a
Compare
related #436