[PATCH] c-csl : accept relative CSL filenames

[PATCH] c-csl : accept relative CSL filenames

[Emmanuel Charpentier]

[-- Attachment #1.1: Type: text/plain, Size: 644 bytes --]

This (minuscule) patch allows to pass a relative (to the buffer's
default directory) file name to denote the CSL style file.

Rationale : this allows the use of "one-of" styles for "one-of"
projects without overloading a defailt CSL style directory (which may
or may not exist on a given system : think JabRef users...). Also
useful for hacked CSL files specific to a given project.

For the same reasons, when the named filename exists both in the
central CSL file and the buffer's default directory, the latter is
retained.

HTH,

PS : Note that I have already transferred my rights to GNU for emacs-
related works.

--
Emmanuel Charpentier


[-- Attachment #1.2: Type: text/html, Size: 862 bytes --]

[-- Attachment #2: 0001-oc-csl-accept-relative-CSL-filenames.patch --]
[-- Type: text/x-patch, Size: 1442 bytes --]

From 80e4121f8a74aec1e5638713ce7af8e041404e44 Mon Sep 17 00:00:00 2001
From: Emmanuel Charpentier <emm.charpentier@free.fr>
Date: Tue, 19 Oct 2021 19:57:17 +0200
Subject: [PATCH] oc-csl : accept relative CSL filenames

---
 lisp/oc-csl.el | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lisp/oc-csl.el b/lisp/oc-csl.el
index 94de97e33..ab75db85b 100644
--- a/lisp/oc-csl.el
+++ b/lisp/oc-csl.el
@@ -364,15 +364,19 @@ corresponding to one of the output formats supported by Citeproc: `html',
 
 INFO is the export state, as a property list.
 
-When file name is relative, expand it according to `org-cite-csl-styles-dir',
+When file name is relative, expand it according to the buffer's default
+directory, failing that according to `org-cite-csl-styles-dir',
 or raise an error if the variable is unset."
   (pcase (org-cite-bibliography-style info)
     ('nil org-cite-csl--fallback-style-file)
     ((and (pred file-name-absolute-p) file) file)
+    ((and (pred (lambda (x)
+		  (let ((fn (expand-file-name x default-directory)))
+		    (if (file-exists-p fn) fn nil)))) file) file)
     ((and (guard org-cite-csl-styles-dir) file)
      (expand-file-name file org-cite-csl-styles-dir))
     (other
-     (user-error "Cannot handle relative style file name: %S" other))))
+     (user-error "CSL style file not found: %S" other))))
 
 (defun org-cite-csl--locale-getter ()
   "Return a locale getter.
-- 
2.33.0

Re: [PATCH] c-csl : accept relative CSL filenames

[Nicolas Goaziou]

Hello,

Emmanuel Charpentier <emm.charpentier@free.fr> writes:

> This (minuscule) patch allows to pass a relative (to the buffer's
> default directory) file name to denote the CSL style file.

Thank you. However, I'm not sure to understand the purpose of the patch.

> Rationale : this allows the use of "one-of" styles for "one-of"
> projects without overloading a defailt CSL style directory (which may
> or may not exist on a given system : think JabRef users...). Also
> useful for hacked CSL files specific to a given project.

If we expand every relative file name from `default-directory', what
happens to `org-cite-csl-styles-dir'? Your patch makes this variable
useless, doesn't it?

What about using a local `org-cite-csl-styles-dir' instead, using
file-local variables?

> +    ((and (pred (lambda (x)
> +		  (let ((fn (expand-file-name x default-directory)))
> +		    (if (file-exists-p fn) fn nil)))) file) file)

This pattern returns the relative file name, not the expanded one. It
may not be what you want.


Regards,
-- 
Nicolas Goaziou

Re: [PATCH] c-csl : accept relative CSL filenames

[Emmanuel Charpentier]

Le mercredi 03 novembre 2021 à 17:19 +0100, Nicolas Goaziou a écrit :
> Hello,
> 
> Emmanuel Charpentier <emm.charpentier@free.fr> writes:
> 
> > This (minuscule) patch allows to pass a relative (to the buffer's
> > default directory) file name to denote the CSL style file.
> 
> Thank you. However, I'm not sure to understand the purpose of the
> patch.
> 
> > Rationale : this allows the use of "one-of" styles for "one-of"
> > projects without overloading a defailt CSL style directory (which may
> > or may not exist on a given system : think JabRef users...). Also
> > useful for hacked CSL files specific to a given project.
> 
> If we expand every relative file name from `default-directory', what
> happens to `org-cite-csl-styles-dir'? Your patch makes this variable
> useless, doesn't it?

This can be useful to be able, for example, to use a one-off style for
a given exporter while keeping a standard one for another exporter (e.
g. a one-off exporter for a journal with complicated bibliographic
requirement and anorher for a preprint archive of the same paper).


> What about using a local `org-cite-csl-styles-dir' instead, using
> file-local variables?


IMHO, `org-cite-csl-styles-dir' is deemed to be a more-or-less
permanent setting (possibly in `.emacs'), whereas allowing a relative
filename doesn't need any initial setup.

> > +    ((and (pred (lambda (x)
> > +                 (let ((fn (expand-file-name x default-directory)))
> > +                   (if (file-exists-p fn) fn nil)))) file) file)
> 
> This pattern returns the relative file name, not the expanded one. It
> may not be what you want.

From `expand-file-name' docstring :

"expand-file-name is a built-in function in ‘C source code’.

(expand-file-name NAME &optional DEFAULT-DIRECTORY)

  Probably introduced at or before Emacs version 1.6.

Convert filename NAME to absolute, and canonicalize it.

[ Abridged.. ]"

My tests confirmed that. I do not understand how you managed to reach
your conclusions...

Sincerely,

--
Emmanuel Charpentier

> 
> 
> Regards,

Re: [PATCH] c-csl : accept relative CSL filenames

[Nicolas Goaziou]

Hello,

Emmanuel Charpentier <emm.charpentier@free.fr> writes:

> IMHO, `org-cite-csl-styles-dir' is deemed to be a more-or-less
> permanent setting (possibly in `.emacs'), whereas allowing a relative
> filename doesn't need any initial setup.

But your patch blocks any use for `org-cite-csl-styles-dir' (relative
file names can always be expanded from `default-directory', the other
branches are thus ignored). So it is either, not both.

I think there are two ways forward when a relative file name is used:

1. First check `org-cite-csl-styles-dir' and, if it is nil, expand
   against `default-directory';

2. Drop `org-cite-csl-styles-dir' and always expand against
   `default-directory'.

 What do oc-csl users think about it?

>> > +    ((and (pred (lambda (x)
>> > +                 (let ((fn (expand-file-name x default-directory)))
>> > +                   (if (file-exists-p fn) fn nil)))) file) file)
>> 
>> This pattern returns the relative file name, not the expanded one. It
>> may not be what you want.
>
> From `expand-file-name' docstring :

I'm not commenting about `expand-file-name', but about your `pcase'
pattern:

  (pcase relative-file-name
    ((and (pred ...) file) file))

IIUC, the above will return `file', which matches `relative-file-name',
not the return value from the predicate.

Regards,
-- 
Nicolas Goaziou

Re: [PATCH] c-csl : accept relative CSL filenames

[Bruce D'Arcus]

On Thu, Nov 4, 2021 at 6:20 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> I think there are two ways forward when a relative file name is used:
>
> 1. First check `org-cite-csl-styles-dir' and, if it is nil, expand
>    against `default-directory';
>
> 2. Drop `org-cite-csl-styles-dir' and always expand against
>    `default-directory'.
>
>  What do oc-csl users think about it?

I'm strongly opposed to 2.

1 seems fine.

Bruce

Re: [PATCH] c-csl : accept relative CSL filenames

[M. ‘quintus’ Gülker]

Am Donnerstag, dem 04. November 2021 schrieb Nicolas Goaziou:
> I think there are two ways forward when a relative file name is used:
>
> 1. First check `org-cite-csl-styles-dir' and, if it is nil, expand
>    against `default-directory';
>
> 2. Drop `org-cite-csl-styles-dir' and always expand against
>    `default-directory'.
>
>  What do oc-csl users think about it?

On the risk that I do not entirely understand the dispute: I would like
to specify relative CSL files. Currently I work on a commentary which
has its own, specific citation guide lines, that is, I need a custom
specific CSL style just for this project. Naturally I store this CSL
file along the .org file in the same directory. I can specify its file
name as an absolute file name (which is what I do currently), but it
would be easier and immune to moving the folder around on my PC if it
could be a relative name. It would also help in creating minimal working
examples for bug reports, which currently require org maintainers to
adapt the path to the CSL file specified.

That would probably mean option 1. Option 2 seems extreme; while I have
not yet accumulated a reasonable collection of CSL files for the German
law journals, I suppose it will happen at some point, in which case it
will come in handy. Still, for the sake of archiving the .org files in a
working state, I may still want to be able to store the CSL file along
with the .org file. After all, the CSL file may be modified later on and
then it may not be possible to re-export the .org file. Having both the
CSL file and the .org file together (in one repository, for instance)
may seem preferable to me. It however is all too new to me to make a
final judgment on this question.

What about a third option: If `org-cite-csl-styles-dir' is not nil,
check if it resolves to an existing file; if no such file exists, expand
against `default-directory'. If `org-cite-csl-styles-dir' is nil, expand
against `default-directory'.

  -quintus

-- 
Dipl.-Jur. M. Gülker | https://mg.guelker.eu | PGP: Siehe Webseite
Passau, Deutschland  | kontakt@guelker.eu    | O<

Re: [PATCH] c-csl : accept relative CSL filenames

[Eric S Fraga]

On Thursday,  4 Nov 2021 at 18:36, Bruce D'Arcus wrote:
> I'm strongly opposed to 2.
>
> 1 seems fine.

I'm with Bruce on this.  I would like to specify a directory for csl
style search that differs from the default directory for Emacs as a
whole.  The latter is already getting rather cluttered these days.

-- 
: Eric S Fraga via Emacs 28.0.60, Org release_9.5-192-gd4e192
: Latest paper written in org: https://arxiv.org/abs/2106.05096

Re: [PATCH] c-csl : accept relative CSL filenames

[Emmanuel Charpentier]

Quintus' example illustrates why I think my proposed patch is useful
(or, rather, will be when corrected :-)), and why the order of research
should be from most-specific to less-specific.

If a finename is not absolute, search :
	1. relatively to the buffer's default directory
	2. if 1. unsuccessfull, relatively to`org-cite-csl-styles-dir'
	3. if 2. unsuccessfull, relatively to emacs' default directory
	   (BTW : what is this ? How to retrieve it ?)
	4. if 3. unsuccessfull, fail.

BTW : my current code does *not* work, and I do not understand why my
test used to succeed... A better patch will follow ( but not
tonight...).

HTH,

--
Emmanuel Charpentier

Re: [PATCH] c-csl : accept relative CSL filenames

[Nicolas Goaziou]

Hello,

Emmanuel Charpentier <emm.charpentier@free.fr> writes:

> If a finename is not absolute, search :
> 	1. relatively to the buffer's default directory
> 	2. if 1. unsuccessfull, relatively to`org-cite-csl-styles-dir'
> 	3. if 2. unsuccessfull, relatively to emacs' default directory
> 	   (BTW : what is this ? How to retrieve it ?)

There's no such thing.

> 	4. if 3. unsuccessfull, fail.

I think `org-cite-csl-styles-dir' trumps buffer default directory as
much as explicit trumps implicit. If you need to override the variable,
you can still use an absolute file name.

I think a better order for a relative file name would be:

 1. relatively to `org-cite-csl-styles-dir',
 2. relatively to buffer's default directory,
 3. failure.

WDYT?
 
Regards,
-- 
Nicolas Goaziou

Re: [PATCH] c-csl : accept relative CSL filenames

[M. ‘quintus’ Gülker]

Am Freitag, dem 19. November 2021 schrieb Nicolas Goaziou:
> I think a better order for a relative file name would be:
>
>  1. relatively to `org-cite-csl-styles-dir',
>  2. relatively to buffer's default directory,
>  3. failure.
>
> WDYT?

I would be fine with it.

  -quintus

-- 
Dipl.-Jur. M. Gülker | https://mg.guelker.eu | PGP: Siehe Webseite
Passau, Deutschland  | kontakt@guelker.eu    | O<

Re: [PATCH] c-csl : accept relative CSL filenames

[Nicolas Goaziou]

Hello,

Emmanuel Charpentier <emm.charpentier@free.fr> writes:

> Quintus' example illustrates why I think my proposed patch is useful
> (or, rather, will be when corrected :-)), and why the order of research
> should be from most-specific to less-specific.
>
> If a finename is not absolute, search :
> 	1. relatively to the buffer's default directory
> 	2. if 1. unsuccessfull, relatively to`org-cite-csl-styles-dir'
> 	3. if 2. unsuccessfull, relatively to emacs' default directory
> 	   (BTW : what is this ? How to retrieve it ?)
> 	4. if 3. unsuccessfull, fail.
>
> BTW : my current code does *not* work, and I do not understand why my
> test used to succeed... A better patch will follow ( but not
> tonight...).

Since you are probably busy, I implemented this on your behalf. The new
behaviour is in main branch. Thank you.

Regards,
-- 
Nicolas Goaziou

Re: [PATCH] c-csl : accept relative CSL filenames

[Kaushal Modi]

Hello Nicolas,

> Since you are probably busy, I implemented this on your behalf. The new
> behaviour is in main branch. Thank you.
>

Can this commit[1] be merged into the main branch. Locally on my
machine, I use org built from main and something like this was working
fine:

#+cite_export: csl cite/csl/ieee.csl

Here, "csl cite/csl/ieee.csl" is the csl path relative to the Org file.

But when the same ran on a CI where the stable Org version is used, it
failed with this error:

> Debugger entered--Lisp error: (user-error "Cannot handle relative style file name: "cite/csl/...")
> signal(user-error ("Cannot handle relative style file name: "cite/csl/..."))
> user-error("Cannot handle relative style file name: %S" "cite/csl/ieee.csl")

I believe the behavior I see with this commit on main branch is kind
of obvious and it should prevent this surprise failure for other users
too.

This is my current workaround for the Org stable version:

#+cite_export: csl ieee.csl

# Local Variables:
# org-cite-csl-styles-dir: "./cite/csl/"
# End:

This works for both main and bugfix, but while doing this, I realized
that even string values are not considered safe for this variable.

Looking through git revisions, I found
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=41e67cff0d3bf27ffb57f9a230598b0385341517.

Earlier `:safe t' was added for `org-cite-csl-styles-dir'. Instead can
we have `:safe #'string-or-null-p' as suggested by Glenn Morris in
that commit?

Thanks!




[1]: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c6186be3fd6c09a6deaa4edc1fbabbad0cb986d3

Re: [PATCH] c-csl : accept relative CSL filenames

[Nicolas Goaziou]

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Can this commit[1] be merged into the main branch.

It is already in the main branch, AFAIU. I assume you mean it should be
back-ported to bugfix branch. If that's the case, I don't know. This is
a new feature, and not a critical one: there are workarounds, as you
found out.

This should be available when Org 9.6 is released. I don't know when
that will happen.

> I believe the behavior I see with this commit on main branch is kind
> of obvious and it should prevent this surprise failure for other users
> too.

It's obvious, but, as a new feature, it was pushed to main instead of
bugfix. Note that the error message is explicit anyway.

I'm not strictly opposed to back-porting it to bugfix, but is there
a compelling reason to break our workflow in this case?

> This works for both main and bugfix, but while doing this, I realized
> that even string values are not considered safe for this variable.
>
> Looking through git revisions, I found
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=41e67cff0d3bf27ffb57f9a230598b0385341517.
n>
> Earlier `:safe t' was added for `org-cite-csl-styles-dir'. Instead can
> we have `:safe #'string-or-null-p' as suggested by Glenn Morris in
> that commit?

I don't think Glenn Morris suggests using #'string-or-null-p, which
would contradict his statement. He is pointing out that ":safe
#'string-or-null-p" is better than ":safe t", even though but allowing
arbitrary locations (strings in this case) is not safe anyway.

Regards,
-- 
Nicolas Goaziou

Re: [PATCH] c-csl : accept relative CSL filenames

[Kaushal Modi]

> It is already in the main branch, AFAIU. I assume you mean it should be
> back-ported to bugfix branch. If that's the case, I don't know. This is
> a new feature, and not a critical one: there are workarounds, as you
> found out.

Yes, sorry, I meant the bugfix branch.

> This should be available when Org 9.6 is released. I don't know when
> that will happen.

OK. It's not urgent. I can wait.

> I don't think Glenn Morris suggests using #'string-or-null-p, which
> would contradict his statement. He is pointing out that ":safe
> #'string-or-null-p" is better than ":safe t", even though but allowing
> arbitrary locations (strings in this case) is not safe anyway.

OK, I thought he referred to ":safe t" equivalent to any arbitrary
location. I don't understand how ":safe
#'string-or-null-p" can be potentially unsafe. So I will go by your judgment.

So, no action needed. Thanks! :)