citations: rx problems with emacs-26.3

citations: rx problems with emacs-26.3

[Maxim Nikulin]

I think citation support is a great feature. (Sorry, I do not work with 
references now, so I can tell nothing concerning implementation for org.)

It seems, rx e.g. in emacs-26.3 does not support all features used in 
oc.el and oc-csl.el. Loading an org file using git master, I get a warning

> Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")

It looks like (rx (regexp ...)) require namely literal, variables are 
not supported yet. In additional (rx (literal ...)) is not supported as 
well.

> Eager macro-expansion failure: (error "Unknown rx form ‘literal’")

In a couple of places `rx-to-string' likely could be used instead of 
just `rx'. I have no idea yet what could be done with (pcase-let* ((rx 
...))) in `org-cite-adjust-note'.

> diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
> index 4e9d2e562..2ac601f34 100644
> --- a/lisp/oc-basic.el
> +++ b/lisp/oc-basic.el
> @@ -656,7 +656,7 @@ present in the citation."
>      (org-open-file file '(4))
>      (if (not (equal "json" (file-name-extension file)))
>          (bibtex-search-entry key)
> -      (let ((regexp (rx "\"id\":" (0+ (any "[ \t]")) "\"" (literal key) "\"")))
> +      (let ((regexp (rx-to-string `(seq "\"id\":" (0+ (any "[ \t]")) "\"" ,key "\"") t)))
>          (goto-char (point-min))
>          (re-search-forward regexp)
>          (search-backward "{")))))
> diff --git a/lisp/oc-csl.el b/lisp/oc-csl.el
> index a7a2a6042..f3c2fd901 100644
> --- a/lisp/oc-csl.el
> +++ b/lisp/oc-csl.el
> @@ -248,10 +248,10 @@ If nil then the Chicago author-date style is used as a fallback.")
>    "Alist mapping locator names to locators.")
>  
>  (defconst org-cite-csl--label-regexp
> -  (rx word-start
> -      (regexp (regexp-opt (mapcar #'car org-cite-csl--label-alist) t))
> -      (0+ digit)
> -      (or word-start line-end (any ?\s ?\t)))
> +  (rx-to-string `(seq word-start
> +                  (regexp ,(regexp-opt (mapcar #'car org-cite-csl--label-alist) t))
> +                  (0+ digit)
> +                  (or word-start line-end (any ?\s ?\t))) t)
>    "Regexp matching a label in a citation reference suffix.
>  Label is in match group 1.")
>  
> diff --git a/lisp/oc.el b/lisp/oc.el
> index 1d704fb03..43a3bb444 100644
> --- a/lisp/oc.el
> +++ b/lisp/oc.el
> @@ -924,7 +924,7 @@ the same object, call `org-cite-adjust-punctuation' first."
>                       (concat final-punct "\"") previous nil nil 2))
>                     (new-next
>                      (replace-regexp-in-string
> -                     (rx string-start (literal final-punct))
> +                     (rx-to-string `(seq string-start ,final-punct) t)
>                       "" next)))
>                 (org-element-set-element previous new-prev)
>                 (org-element-set-element next new-next)

Maybe I have missed other similar problems.

Does someone have better ideas how to restore compatibility with older 
emacs versions? 24.3 is declared in org.el. Ubuntu 20.04 (latest long 
time support release) has 26.3.

Re: citations: rx problems with emacs-26.3

[Nicolas Goaziou]

Hello,

Maxim Nikulin <manikulin@gmail.com> writes:

> I think citation support is a great feature. (Sorry, I do not work
> with references now, so I can tell nothing concerning implementation
> for org.)
>
> It seems, rx e.g. in emacs-26.3 does not support all features used in
> oc.el and oc-csl.el. Loading an org file using git master, I get
> a warning
>
>> Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")
>
> It looks like (rx (regexp ...)) require namely literal, variables are
> not supported yet. In additional (rx (literal ...)) is not supported
> as well.
>
>> Eager macro-expansion failure: (error "Unknown rx form ‘literal’")
>
> In a couple of places `rx-to-string' likely could be used instead of
> just `rx'. I have no idea yet what could be done with (pcase-let* ((rx 
> ...))) in `org-cite-adjust-note'.
>
>> diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el

Thanks. Could you send the patch again with a proper commit message,
using git format-patch?

Regards,
-- 
Nicolas Goaziou

Re: citations: rx problems with emacs-26.3

[Maxim Nikulin]

On 08/08/2021 03:27, Nicolas Goaziou wrote:
> Maxim Nikulin writes:
>>
>> It seems, rx e.g. in emacs-26.3 does not support all features used in
>> oc.el and oc-csl.el. Loading an org file using git master, I get
>> a warning
>>
>>> Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")
>>
>>> diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
> 
> Thanks. Could you send the patch again with a proper commit message,
> using git format-patch?

I going to do it. For a while, I have noticed another problem with a lot 
of pcase branches in `org-cite-natbib--style-to-command' in and my 
experience is not enough to resolve it. On attempt to byte-compile-file
the following error is reported after significant delay (and CPU fan 
becomes more noisy)

oc-natbib.el:108:9:Error: Bytecode overflow

I have tried to comment out some conditions

ls -l lisp/oc-natbib.elc
-rw-rw-r-- 1 ubuntu ubuntu 241882 Aug  8 13:23 lisp/oc-natbib.elc

If more branches are commented out

-rw-rw-r-- 1 ubuntu ubuntu 8525 Aug  8 13:20 lisp/oc-natbib.el

Looks like a kind of combinatorics explosion. It seems, if Emacs-25.2 
and 26.3 are considered as supported, this function should be rewritten 
to avoid issues with byte compilation.

Re: citations: rx problems with emacs-26.3

[Nicolas Goaziou]

Hello,

Maxim Nikulin <manikulin@gmail.com> writes:

> On 08/08/2021 03:27, Nicolas Goaziou wrote:
>> Maxim Nikulin writes:
>>>
>>> It seems, rx e.g. in emacs-26.3 does not support all features used in
>>> oc.el and oc-csl.el. Loading an org file using git master, I get
>>> a warning
>>>
>>>> Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")
>>>
>>>> diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
>> Thanks. Could you send the patch again with a proper commit message,
>> using git format-patch?
>
> I going to do it. For a while, I have noticed another problem with
> a lot of pcase branches in `org-cite-natbib--style-to-command' in and
> my experience is not enough to resolve it. On attempt to
> byte-compile-file
> the following error is reported after significant delay (and CPU fan
> becomes more noisy)
>
> oc-natbib.el:108:9:Error: Bytecode overflow

Fixed. Thank you.

Regards,
-- 
Nicolas Goaziou

Re: citations: rx problems with emacs-26.3

[Maxim Nikulin]

On 08/08/2021 21:14, Nicolas Goaziou wrote:
> Maxim Nikulin writes:
> 
>> I have noticed another problem with
>> a lot of pcase branches in `org-cite-natbib--style-to-command' in and
>> my experience is not enough to resolve it. On attempt to
>> byte-compile-file
>> the following error is reported after significant delay (and CPU fan
>> becomes more noisy)
>>
>> oc-natbib.el:108:9:Error: Bytecode overflow
> 
> Fixed. Thank you.

Nice. Perhaps `org-cite-biblatex-export-citation' should be fixed in a 
similar way. There is no error yet but compiled file is significantly 
blown up:

-rw-rw-r-- 1 ubuntu ubuntu 13383 Aug  9 02:17 lisp/oc-biblatex.el
-rw-rw-r-- 1 ubuntu ubuntu 48906 Aug  9 17:06 lisp/oc-biblatex.elc

There are a couple of issues with "make single" for emacs-25.2:

Compiling single /home/ubuntu/org-mode/lisp/oc-basic.el...

In end of data:
oc-basic.el:772:1:Warning: the function ‘buffer-hash’ is not known to be
     defined.
Compiling single /home/ubuntu/org-mode/lisp/oc-biblatex.el...
Compiling single /home/ubuntu/org-mode/lisp/oc-csl.el...

In toplevel form:
oc-csl.el:90:1:Error: Cannot open load file: No such file or directory, 
org-cite
Makefile:59: recipe for target 'oc-csl.elc' failed
make[2]: [oc-csl.elc] Error 1 (ignored)

Re: citations: rx problems with emacs-26.3

[Axel Kielhorn]

> Am 09.08.2021 um 19:18 schrieb Maxim Nikulin <manikulin@gmail.com>:
> 
> On 08/08/2021 21:14, Nicolas Goaziou wrote:
>> Maxim Nikulin writes:
>>> I have noticed another problem with
>>> a lot of pcase branches in `org-cite-natbib--style-to-command' in and
>>> my experience is not enough to resolve it. On attempt to
>>> byte-compile-file
>>> the following error is reported after significant delay (and CPU fan
>>> becomes more noisy)
>>> 
>>> oc-natbib.el:108:9:Error: Bytecode overflow
>> Fixed. Thank you.
> 
> Nice. Perhaps `org-cite-biblatex-export-citation' should be fixed in a similar way. There is no error yet but compiled file is significantly blown up:
> 
> -rw-rw-r-- 1 ubuntu ubuntu 13383 Aug  9 02:17 lisp/oc-biblatex.el
> -rw-rw-r-- 1 ubuntu ubuntu 48906 Aug  9 17:06 lisp/oc-biblatex.elc
> 
> There are a couple of issues with "make single" for emacs-25.2:

There are still problems with Emacs 26.3

From making up1 from todays 01877f480:

Compiling /Users/axel/src/Emacs/org-mode/lisp/ob-C.el...
Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")

In toplevel form:
oc-basic.el:648:1:Error: Unknown rx form ‘literal’
Compiling /Users/axel/src/Emacs/org-mode/lisp/oc-biblatex.el...

In toplevel form:
oc-biblatex.el:248:1:Error: Unknown rx form ‘anychar’
Compiling /Users/axel/src/Emacs/org-mode/lisp/oc-csl.el...

In toplevel form:
oc-csl.el:255:1:Error: rx form ‘regexp’ requires args satisfying ‘stringp’

In toplevel form:
oc.el:866:1:Error: rx form ‘regexp’ requires args satisfying ‘stringp’
In end of data:
org-compat.el:1255:1:Warning: the function ‘directory-empty-p’ is not known to
    be defined.

Eager macro-expansion failure: (error "Unknown rx form ‘literal’“)

The patch from Maxim Nikulin fixes these for me.

Greetings Axel

Re: citations: rx problems with emacs-26.3

[Nicolas Goaziou]

Hello,

Maxim Nikulin <manikulin@gmail.com> writes:

> Nice. Perhaps `org-cite-biblatex-export-citation' should be fixed in
> a similar way. There is no error yet but compiled file is
> significantly blown up:
>
> -rw-rw-r-- 1 ubuntu ubuntu 13383 Aug  9 02:17 lisp/oc-biblatex.el
> -rw-rw-r-- 1 ubuntu ubuntu 48906 Aug  9 17:06 lisp/oc-biblatex.elc
>
> There are a couple of issues with "make single" for emacs-25.2:
>
> Compiling single /home/ubuntu/org-mode/lisp/oc-basic.el...
>
> In end of data:
> oc-basic.el:772:1:Warning: the function ‘buffer-hash’ is not known to be
>     defined.
> Compiling single /home/ubuntu/org-mode/lisp/oc-biblatex.el...
> Compiling single /home/ubuntu/org-mode/lisp/oc-csl.el...
>
> In toplevel form:
> oc-csl.el:90:1:Error: Cannot open load file: No such file or
> directory, org-cite
> Makefile:59: recipe for target 'oc-csl.elc' failed
> make[2]: [oc-csl.elc] Error 1 (ignored)

All fixed. Thank you.

Regards,
-- 
Nicolas Goaziou

Re: citations: rx problems with emacs-26.3

[Maxim Nikulin]

On 11/08/2021 16:33, Nicolas Goaziou wrote:
> Maxim Nikulin writes:
> 
>> There are a couple of issues with "make single" for emacs-25.2:
> 
> All fixed. Thank you.

Thank you, Nicolas. It seems org-cite can be cleanly loaded or compiled 
by Emacs-25 and Emacs-26 now.

Re: citations: rx problems with emacs-26.3

[Maxim Nikulin]

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

On 08/08/2021 03:27, Nicolas Goaziou wrote:
> 
> Maxim Nikulin writes:
> 
>> It seems, rx e.g. in emacs-26.3 does not support all features used in
>> oc.el and oc-csl.el. Loading an org file using git master, I get
>> a warning
>>
>>> Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")
> 
> Thanks. Could you send the patch again with a proper commit message,
> using git format-patch?

With the attached patches I do not see warnings any more while org is 
loading. On the other hand I am not an org-cite user, so I am not sure 
that nothing is broken by these patches.



[-- Attachment #2: 0001-oc-rx-compatibility-with-Emacs-27.1.patch --]
[-- Type: text/x-patch, Size: 5207 bytes --]

From 151191d0283cb55e719c9c01459de02dfb584e02 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Mon, 9 Aug 2021 23:59:41 +0700
Subject: [PATCH 1/2] oc: rx compatibility with Emacs < 27.1

* lisp/oc.el (org-cite-adjust-note):
* lisp/oc-basic.el (org-cite-basic-goto):
* lisp/oc-biblatex.el (org-cite-biblatex-prepare-preamble):
* lisp/oc-csl.el (org-cite-csl--label-regexp): Avoid `rx' forms
introduced in Emacs-27.1 to keep compatibility with older versions.

Use `rx-to-string' instead of `rx' due to lack of support of  `regexp'
and `literal' forms with a variable as an argument.
---
 lisp/oc-basic.el    |  4 +++-
 lisp/oc-biblatex.el |  2 +-
 lisp/oc-csl.el      | 11 +++++++----
 lisp/oc.el          | 21 ++++++++++++++-------
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index c46e7a715..6d625e4a5 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -667,7 +667,9 @@ present in the citation."
     (org-open-file file '(4))
     (pcase (file-name-extension file)
       ("json"
-       (let ((regexp (rx "\"id\":" (0+ (any "[ \t]")) "\"" (literal key) "\"")))
+       ;; `rx' can not be used with Emacs <27.1 since `literal' form
+       ;; is not supported.
+       (let ((regexp (rx-to-string `(seq "\"id\":" (0+ (any "[ \t]")) "\"" ,key "\"") t)))
          (goto-char (point-min))
          (re-search-forward regexp)
          (search-backward "{")))
diff --git a/lisp/oc-biblatex.el b/lisp/oc-biblatex.el
index 5a9c304a3..c82a77046 100644
--- a/lisp/oc-biblatex.el
+++ b/lisp/oc-biblatex.el
@@ -260,7 +260,7 @@ to the document, and set styles."
       ;; Then set options.
       (goto-char (match-beginning 0))
       (let ((re (rx "\\usepackage"
-                    (opt (group "[" (*? anychar) "]"))
+                    (opt (group "[" (*? anything) "]"))
                     "{biblatex}")))
         (cond
          ;; No "biblatex" package loaded.  Insert "usepackage" command
diff --git a/lisp/oc-csl.el b/lisp/oc-csl.el
index 5e0e27211..c08adc430 100644
--- a/lisp/oc-csl.el
+++ b/lisp/oc-csl.el
@@ -253,10 +253,13 @@ If nil then the Chicago author-date style is used as a fallback.")
   "Alist mapping locator names to locators.")
 
 (defconst org-cite-csl--label-regexp
-  (rx word-start
-      (regexp (regexp-opt (mapcar #'car org-cite-csl--label-alist) t))
-      (0+ digit)
-      (or word-start line-end (any ?\s ?\t)))
+  ;; Prior to Emacs-27.1 argument of `regexp' form must be a string literal.
+  ;; It is the reason why `rx' is avoided here.
+  (rx-to-string `(seq word-start
+                  (regexp ,(regexp-opt (mapcar #'car org-cite-csl--label-alist) t))
+                  (0+ digit)
+                  (or word-start line-end (any ?\s ?\t)))
+                t)
   "Regexp matching a label in a citation reference suffix.
 Label is in match group 1.")
 
diff --git a/lisp/oc.el b/lisp/oc.el
index 338348116..53eafb3bd 100644
--- a/lisp/oc.el
+++ b/lisp/oc.el
@@ -878,14 +878,19 @@ the same object, call `org-cite-adjust-punctuation' first."
   (when org-cite-adjust-note-numbers
     (pcase-let* ((rule (or rule (org-cite--get-note-rule info)))
                  (punct-re (regexp-opt (or punct org-cite-punctuation-marks)))
+                 ;; with Emacs <27.1. Argument of `regexp' form (PUNCT-RE this case)
+                 ;; must be a string literal.
                  (previous-punct-re
-                  (rx (opt (group (0+ (any blank ?\n)) (regexp punct-re)))
-                      (opt (0+ (any blank ?\n)) (group ?\"))
-                      (opt (group (1+ (any blank ?\n))))
-                      string-end))
+                  (rx-to-string `(seq (opt (group (regexp ,(rx (0+ (any blank ?\n))))
+                                                  (regexp ,punct-re)))
+                                      (regexp ,(rx (opt (0+ (any blank ?\n)) (group ?\"))
+                                                   (opt (group (1+ (any blank ?\n))))
+                                                   string-end)))
+                                t))
                  (next-punct-re
-                  (rx string-start
-                      (group (0+ (any blank ?\n)) (regexp punct-re))))
+                  (rx-to-string `(seq string-start
+                                      (group (0+ (any blank ?\n)) (regexp ,punct-re)))
+                                t))
                  (next (org-export-get-next-element citation info))
                  (final-punct
                   (and (stringp next)
@@ -928,7 +933,9 @@ the same object, call `org-cite-adjust-punctuation' first."
                      (concat final-punct "\"") previous nil nil 2))
                    (new-next
                     (replace-regexp-in-string
-                     (rx string-start (literal final-punct))
+                     ;; Before Emacs-27.1 `literal' `rx' form with a variable
+                     ;; as an argument is not available.
+                     (rx-to-string `(seq string-start ,final-punct) t)
                      "" next)))
                (org-element-set-element previous new-prev)
                (org-element-set-element next new-next)
-- 
2.25.1


[-- Attachment #3: 0002-oc-Add-missing-require.patch --]
[-- Type: text/x-patch, Size: 1015 bytes --]

From 19261fcf089deba09250117fc1beb6a8aef69450 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Tue, 10 Aug 2021 00:03:25 +0700
Subject: [PATCH 2/2] oc: Add missing `require'

* lisp/oc.el:
* lisp/oc-basic.el: Add some `require' to avoid compile warnings.
---
 lisp/oc-basic.el | 3 +++
 lisp/oc.el       | 1 +
 2 files changed, 4 insertions(+)

diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index 6d625e4a5..66f8c069f 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -65,7 +65,10 @@
 ;;; Code:
 
 (require 'bibtex)
+(require 'json)
 (require 'oc)
+(require 'org)
+(require 'seq)
 
 (declare-function org-open-at-point "org" (&optional arg))
 
diff --git a/lisp/oc.el b/lisp/oc.el
index 53eafb3bd..f8cc7829c 100644
--- a/lisp/oc.el
+++ b/lisp/oc.el
@@ -57,6 +57,7 @@
 
 (require 'org-compat)
 (require 'org-macs)
+(require 'seq)
 
 (declare-function org-at-heading-p "org" (&optional _))
 (declare-function org-collect-keywords "org" (keywords &optional unique directory))
-- 
2.25.1

Re: citations: rx problems with emacs-26.3

[Nicolas Goaziou]

Hello,

Maxim Nikulin <manikulin@gmail.com> writes:

> On 08/08/2021 03:27, Nicolas Goaziou wrote:
>> Maxim Nikulin writes:
>> 
>>> It seems, rx e.g. in emacs-26.3 does not support all features used in
>>> oc.el and oc-csl.el. Loading an org file using git master, I get
>>> a warning
>>>
>>>> Eager macro-expansion failure: (error "rx form ‘regexp’ requires args satisfying ‘stringp’")
>> Thanks. Could you send the patch again with a proper commit message,
>> using git format-patch?
>
> With the attached patches I do not see warnings any more while org is
> loading. On the other hand I am not an org-cite user,

Applied. Thank you.

> so I am not sure that nothing is broken by these patches.

I'll trust the test suite on this.

Regards,
-- 
Nicolas Goaziou