More robust CSL-JSON date parsing in oc-basic

More robust CSL-JSON date parsing in oc-basic

[David Lukeš]

Hi all,

I think I've found an opportunity to make oc-basic a bit more resilient
w.r.t. different kinds of CSL-JSON publication date input. I encountered
the following error:

  Error (org-mode-hook): Error running hook "org-fancy-priorities-mode"
  because: (error Unknown CSL-JSON date format: nil)

(Based on the debugger traceback, what triggered it was citar loading
oc-basic for fontification purposes.)

After a small tweak to oc-basic (see below), the error message got a bit
more informative:

  Error (org-mode-hook): Error running hook "org-fancy-priorities-mode"
  because: (error Unknown CSL-JSON date format: ((literal . "2009 / 09 /
  01 /")))

Based on this, I ended up making the following changes to oc-basic:

--8<---------------------------------------------------------------->8--

diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index fc71d22fc..de9be40a1 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -178,21 +178,24 @@ Return a hash table with citation references as
keys and fields alist as values.
                                      " and ")))
                              ('issued
                               ;; Date are expressed as an array
-                              ;; (`date-parts') or a "string (`raw').
-                              ;; In both cases, extract the year and
-                              ;; associate it to `year' field, for
-                              ;; compatibility with BibTeX format.
+                              ;; (`date-parts') or a "string (`raw'
+                              ;; or `literal'). In both cases,
+                              ;; extract the year and associate it
+                              ;; to `year' field, for compatibility
+                              ;; with BibTeX format.
                               (let ((date (or (alist-get 'date-parts value)
-                                              (alist-get 'raw value))))
+                                              (alist-get 'raw value)
+                                              (alist-get 'literal value))))
                                 (cons 'year
                                       (cond
                                        ((consp date)
                                         (caar date))
                                        ((stringp date)
-                                        (car (split-string date "-")))
+                                        (replace-regexp-in-string
".*?\\([0-9]\\{4\\}\\).*" "\\1" date))
+
                                        (t
                                         (error "Unknown CSL-JSON date
format: %S"
-                                               date))))))
+                                               value))))))
                              (_
                               (cons field value))))
                          item)

--8<---------------------------------------------------------------->8--

A few comments, starting from the top:

1. In practice, it looks like string-valued dates can either be tagged
   with 'raw or with 'literal. Not sure what the difference is, whether
   one is older or deprecated. I just know that my CSL-JSON (produced by
   Better BibTeX from Zotero) is full of the latter, and oc-basic didn't
   account for this possibility, so I added it.
2. I'm pretty sure the weird date format, "2009 / 09 / 01 /", is not
   something I entered manually. My preferred format would be
   "2009-09-01", which is what oc-basic currently expects. But with
   Zotero, I tend to rely on metadata being filled in automagically, and
   it looks like it's the Wild West out there. So I figured a more
   robust way to extract the year portion might be to match the first
   sequence of 4 digits, instead of splitting the string on - and taking
   the car of the resulting list.

   Of course, both are just heuristics, both can fail (each in different
   ways), and both can result in the entire string being used as the
   year. My suggested change is based purely on the fact that the
   4-digit heuristic more often does the right thing in my own
   collection of bibliography entries.
3. The last change uses value instead of date in the error message,
   which makes it more informative, as shown above -- when encountering
   a problem, the user gets to see the offending entry, instead of just
   nil.

Let me know which of these changes -- if any -- seem acceptable, and
whether they qualify as tinychange, and I'll happily submit a proper
patch :)

Best,

David

Re: More robust CSL-JSON date parsing in oc-basic

[Nicolas Goaziou]

Hello,

David Lukeš <dafydd.lukes@gmail.com> writes:

> -                                        (car (split-string date "-")))
> +                                        (replace-regexp-in-string
> ".*?\\([0-9]\\{4\\}\\).*" "\\1" date))
> +

[...]

> Let me know which of these changes -- if any -- seem acceptable, and
> whether they qualify as tinychange, and I'll happily submit a proper
> patch :)

All changes sound good. Please use `rx' for the regexp matching the
year.

This is a tinychange. I think this can be applied to the bugfix branch.

Thanks!

Regards,
-- 
Nicolas Goaziou

Re: More robust CSL-JSON date parsing in oc-basic

[David Lukeš]

[-- Attachment #1: Type: text/plain, Size: 78 bytes --]

Thanks for the quick feedback! I'll send the patch momentarily.

Best,

David

[-- Attachment #2: Type: text/html, Size: 166 bytes --]