GNU bug report logs - #44647
27.1.50; `bibtex-contline-indentation' doesn't work as file local variable

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44647 in the body.
You can then email your comments to 44647 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.1.50; `bibtex-contline-indentation' doesn't work as file local
 variable
Date: Sat, 14 Nov 2020 23:18:16 +0200

[Message part 1 (text/plain, inline)]

Variable bibtex-contline-indentation does not work as file local
variable. The reason is that bibtex-mode command initializes variable
fill-prefix before the possible file local variable is available. It
gets always the global value of bibtex-contline-indentation.

This can be fixed by locally let-binding fill-prefix every time in the
relevant filling function. Patch for that is attached.

[0001-Make-bibtex-contline-indentation-work-as-file-local-.patch (text/x-diff, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 44647 <at> debbugs.gnu.org (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: winkler <at> gnu.org
Cc: 44647 <at> debbugs.gnu.org
Subject: Re: 27.1.50; `bibtex-contline-indentation' doesn't work as file
 local variable
Date: Sat, 14 Nov 2020 23:27:41 +0200

[Message part 1 (text/plain, inline)]

Hello Roland!

I found and fixed a bug in Emacs bibtex.el file. Here's my description:

* 2020-11-14 23:18:16+02, Teemu Likonen wrote:
> Variable bibtex-contline-indentation does not work as file local
> variable. The reason is that bibtex-mode command initializes variable
> fill-prefix before the possible file local variable is available. It
> gets always the global value of bibtex-contline-indentation.
>
> This can be fixed by locally let-binding fill-prefix every time in the
> relevant filling function. Patch for that is attached.

See: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44647

What do you think of the patch which I attached to the bug report?

-- 
/// Teemu Likonen - .-.. https://www.iki.fi/tlikonen/
// OpenPGP: 4E1055DC84E9DFF613D78557719D69D324539450

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 44647 <at> debbugs.gnu.org (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: winkler <at> gnu.org
Cc: 44647 <at> debbugs.gnu.org
Subject: Re: 27.1.50; `bibtex-contline-indentation' doesn't work as file
 local variable
Date: Sun, 15 Nov 2020 11:39:59 +0200

[Message part 1 (text/plain, inline)]

* 2020-11-14 23:27:41+02, Teemu Likonen wrote:
> * 2020-11-14 23:18:16+02, Teemu Likonen wrote:
>> Variable bibtex-contline-indentation does not work as file local
>> variable. The reason is that bibtex-mode command initializes variable
>> fill-prefix before the possible file local variable is available. It
>> gets always the global value of bibtex-contline-indentation.
>>
>> This can be fixed by locally let-binding fill-prefix every time in the
>> relevant filling function. Patch for that is attached.
>
> See: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44647

I'll add one example. Currently if we want to use file or directory
local variable to define bibtex indentation we must do use variable
bibtex-text-indentation for normal field value indentation and then use
fill-prefix with spaces to define the indentation for continuation
lines:

    ;; .dir-locals.el
    ((bibtex-mode . ((bibtex-text-indentation . 20)
                     (fill-prefix . "                     ")))) ; 21 spaces

Quite obviously more user-friendly way would be this:

    ;; .dir-locals.el
    ((bibtex-mode . ((bibtex-text-indentation . 20)
                     (bibtex-contline-indentation . 21))))

My patch does this. It let-binds fill-prefix every time and initializes
its value by using the (possibly buffer local) value of
bibtex-contline-indentation.

Here is inline version of the patch so that it is easier to read in
debbugs.gnu.org page.

[0001-Make-bibtex-contline-indentation-work-as-file-local-.patch (text/x-diff, inline)]

From a776cae4fcd34987e30b6eab3df45bd2ae66fbd9 Mon Sep 17 00:00:00 2001
From: Teemu Likonen <tlikonen <at> iki.fi>
Date: Sat, 14 Nov 2020 22:53:18 +0200
Subject: [PATCH] Make `bibtex-contline-indentation' work as file local
 variable

* lisp/textmodes/bibtex.el (bibtex-mode): Don't make fill-prefix a
buffer local variable. It would use the global value, not possible
file local variable.
* lisp/textmodes/bibtex.el (bibtex-fill-field-bounds): Let-bind
fill-prefix variable locally (and dynamically) for use with filling
bibtex fields. The variable is initialized every time by using the
current value of bibtex-contline-indentation variable.
---
 lisp/textmodes/bibtex.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/textmodes/bibtex.el b/lisp/textmodes/bibtex.el
index fcf63ed5ec..151dca6fab 100644
--- a/lisp/textmodes/bibtex.el
+++ b/lisp/textmodes/bibtex.el
@@ -3442,8 +3442,6 @@ bibtex-mode
   (set (make-local-variable 'defun-prompt-regexp) "^[ \t]*@[[:alnum:]]+[ \t]*")
   (set (make-local-variable 'outline-regexp) "[ \t]*@")
   (set (make-local-variable 'fill-paragraph-function) #'bibtex-fill-field)
-  (set (make-local-variable 'fill-prefix)
-       (make-string (+ bibtex-entry-offset bibtex-contline-indentation) ?\s))
   (set (make-local-variable 'font-lock-defaults)
        '(bibtex-font-lock-keywords
          nil t ((?$ . "\"")
@@ -4902,7 +4900,10 @@ bibtex-fill-field-bounds
   "Fill BibTeX field delimited by BOUNDS.
 If JUSTIFY is non-nil justify as well.
 If optional arg MOVE is non-nil move point to end of field."
-  (let ((end-field (copy-marker (bibtex-end-of-field bounds))))
+  (let ((end-field (copy-marker (bibtex-end-of-field bounds)))
+        (fill-prefix (make-string (+ bibtex-entry-offset
+                                     bibtex-contline-indentation)
+                                  ?\s)))
     (if (not justify)
         (goto-char (bibtex-start-of-text-in-field bounds))
       (goto-char (bibtex-start-of-field bounds))
-- 
2.20.1

[Message part 3 (text/plain, inline)]

-- 
/// Teemu Likonen - .-.. https://www.iki.fi/tlikonen/
// OpenPGP: 4E1055DC84E9DFF613D78557719D69D324539450

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 44647 <at> debbugs.gnu.org (full text, mbox):

From: "Roland Winkler" <winkler <at> gnu.org>
To: Teemu Likonen <tlikonen <at> iki.fi>
Cc: 44647 <at> debbugs.gnu.org
Subject: Re: 27.1.50; `bibtex-contline-indentation' doesn't work as file
 local variable
Date: Sun, 15 Nov 2020 22:09:39 -0600

[Message part 1 (text/plain, inline)]

On Sat Nov 14 2020 Teemu Likonen wrote:
> This can be fixed by locally let-binding fill-prefix every time in
> the relevant filling function. Patch for that is attached.

Your patch implies that the buffer-local value of fill-prefix is not
what it should be (say, if any other command wants to use it).

How about the rather different patch attached below, partly inspired
by the related comment

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21764#15

which had somehow escaped my attention.  The issues mentioned in
this comment should likewise be fixed by the attached patch.

[bibtex.diff (application/octet-stream, attachment)]

Message #17 received at 44647 <at> debbugs.gnu.org (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: Roland Winkler <winkler <at> gnu.org>
Cc: 44647 <at> debbugs.gnu.org
Subject: Re: 27.1.50; `bibtex-contline-indentation' doesn't work as file
 local variable
Date: Mon, 16 Nov 2020 16:05:18 +0200

[Message part 1 (text/plain, inline)]

* 2020-11-15 22:09:39-06, Roland Winkler wrote:

> On Sat Nov 14 2020 Teemu Likonen wrote:
>> This can be fixed by locally let-binding fill-prefix every time in
>> the relevant filling function. Patch for that is attached.
>
> Your patch implies that the buffer-local value of fill-prefix is not
> what it should be (say, if any other command wants to use it).

Yes. Bibtex-mode's filling uses fill-prefix and that variable must be
set to correct value before filling. The correct moment may not be when
a bibtex-mode buffer is initialized with buffer-local values. More about
this below.

> How about the rather different patch attached below, partly inspired
> by the related comment
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21764#15
>
> which had somehow escaped my attention.  The issues mentioned in
> this comment should likewise be fixed by the attached patch.

That patch sets buffer-local value for fill-prefix correctly. I'm quite
okay with that but it is not optimal. If user later changes the related
settings, like

    (setq-local bibtex-text-indentation 25
                bibtex-contline-indentation 26)

and then tries to fill the current bibtex entry (C-c C-q) then only
bibtex-text-indentation actually works but bibtex-contline-indentation
doesn't have any effect. The wrong result is caused by the wrong value
of fill-prefix, like this:

    @book{pitkäjohdanto,
      author =               {Oetiker, Tobias and Partl, Hubert and Hyna, Irene and
                      Schlegl, Elisabeth and Hell\-gren, Timo},
      title =                {Pitkänpuoleinen johdanto Latex 2ε:n käyttöön},
      subtitle =             {Eli opi Latex 2ε 133 minuutissa},
      note =                 {Versio 4.17fi, lokakuu 2005},
      date =                 2005,
      url =                  {https://www.ctan.org/pkg/lshort-finnish},
    }

That is why I think fill-prefix should be let-bound every time. User
should be able to trust that bibtex-contline-indentation does the right
thing.

-- 
/// Teemu Likonen - .-.. https://www.iki.fi/tlikonen/
// OpenPGP: 4E1055DC84E9DFF613D78557719D69D324539450

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 44647 <at> debbugs.gnu.org (full text, mbox):

From: "Roland Winkler" <winkler <at> gnu.org>
To: Teemu Likonen <tlikonen <at> iki.fi>
Cc: 44647 <at> debbugs.gnu.org
Subject: Re: 27.1.50; `bibtex-contline-indentation' doesn't work as file
 local variable
Date: Mon, 16 Nov 2020 10:36:43 -0600

On Mon Nov 16 2020 Teemu Likonen wrote:
> > Your patch implies that the buffer-local value of fill-prefix is not
> > what it should be (say, if any other command wants to use it).
> 
> Yes. Bibtex-mode's filling uses fill-prefix and that variable must
> be set to correct value before filling. The correct moment may not
> be when a bibtex-mode buffer is initialized with buffer-local
> values.

Defcustom supports the keyword :set that can handle the case when a
user variable such as bibtex-contline-indentation is changed which
implies updating another variable such as fill-prefix.  However, I
assume that this is a rather rare case in the present context.

On the other hand, it is a design principle of emacs that variables
such as fill-prefix can be and are used by different commands.  So
they should have their proper value instead of only let-binding them
on the fly.  Such let-bindings are used only when it is the intent
to use different values than the proper buffer-local values.

Message #23 received at 44647 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Teemu Likonen <tlikonen <at> iki.fi>
Cc: 44647 <at> debbugs.gnu.org, winkler <at> gnu.org
Subject: Re: bug#44647: 27.1.50; `bibtex-contline-indentation' doesn't work
 as file local variable
Date: Mon, 16 Nov 2020 23:58:12 +0100

Teemu Likonen <tlikonen <at> iki.fi> writes:

> I found and fixed a bug in Emacs bibtex.el file. Here's my description:

This is the same as bug#44618, so I'm merging these two.  See my
comments there.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #28 received at 44647-done <at> debbugs.gnu.org (full text, mbox):

From: "Roland Winkler" <winkler <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: pot <at> gnu.org, Teemu Likonen <tlikonen <at> iki.fi>, 44647-done <at> debbugs.gnu.org,
 44618-done <at> debbugs.gnu.org
Subject: Re: bug#44618: 26.3; bibtex.el ignores file variable
 bibtex-contline-indentation
Date: Wed, 2 Dec 2020 12:38:19 -0600

On Tue Nov 17 2020 Lars Ingebrigtsen wrote:
> If I'm reading the patch correctly, it looks good to me -- it'll still
> allow people to set fill-prefix from bibtex-mode-hook, and it fixes the
> reported problem in these two bug reports.  (I haven't tested the patch,
> though, just read it.)

Installed as patch d9167d940ac9d0504cc38a044c7cba897c103eaf.
I am closing both bug#44618 and bug#44647.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.