unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation
@ 2020-11-13 12:45 Francesco Potortì
  2020-11-14 16:20 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 6+ messages in thread
From: Francesco Potortì @ 2020-11-13 12:45 UTC (permalink / raw)
  To: 44618

In GNU Emacs 26.3 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2020-05-17, modified by Debian built on x86-csail-01

I set bibtex-contline-indentation as a file variable, yeet it is ignored
by bibtex-mode.

I am not sure how file variables and minor modes interact, but it looks
like when the minor mode is fired, the local variables have not been
set yet.

Whatever the reason, bibtex-mode sets fill-prefix as a local variable
irrespective of the file variable bibtex-contline-indentation.

The fix for my case is simple: avoid setting fill-prefix as a local
variable in the minor mode, and just set it inside the
bibtex-fill-field-bounds function.  A patch follows.

However, I don't know if this is a hint of some more general problem on
how bibtex-mode trets file variables.

--- bibtex-2019.el	2020-11-13 11:54:34.000000000 +0100
+++ bibtex.el	2020-11-13 13:34:24.000000000 +0100
@@ -3399,8 +3409,6 @@
   (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 ((?$ . "\"")
@@ -4830,7 +4838,9 @@
   "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 ((fill-prefix
+	 (make-string (+ bibtex-entry-offset bibtex-contline-indentation) ?\s))
+	(end-field (copy-marker (bibtex-end-of-field bounds))))
     (if (not justify)
         (goto-char (bibtex-start-of-text-in-field bounds))
       (goto-char (bibtex-start-of-field bounds))





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation
  2020-11-13 12:45 bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation Francesco Potortì
@ 2020-11-14 16:20 ` Lars Ingebrigtsen
  2020-11-16 23:31   ` Roland Winkler
  0 siblings, 1 reply; 6+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-14 16:20 UTC (permalink / raw)
  To: Francesco Potortì; +Cc: 44618

Francesco Potortì <pot@gnu.org> writes:

> Whatever the reason, bibtex-mode sets fill-prefix as a local variable
> irrespective of the file variable bibtex-contline-indentation.
>
> The fix for my case is simple: avoid setting fill-prefix as a local
> variable in the minor mode, and just set it inside the
> bibtex-fill-field-bounds function.  A patch follows.
>
> However, I don't know if this is a hint of some more general problem on
> how bibtex-mode trets file variables.

[...]

> -  (set (make-local-variable 'fill-prefix)
> -       (make-string (+ bibtex-entry-offset bibtex-contline-indentation) ?\s))

[...]

> -  (let ((end-field (copy-marker (bibtex-end-of-field bounds))))
> +  (let ((fill-prefix
> +	 (make-string (+ bibtex-entry-offset bibtex-contline-indentation) ?\s))
> +	(end-field (copy-marker (bibtex-end-of-field bounds))))

This patch would make it impossible for people to alter fill-prefix
(from the mode hook, for instance), so I don't think this is the right
solution.

Looking at hack-local-variables/normal-mode, file-local variables are
set after the mode has been set up, so you can't have your mode set up
variables based on file-local variables.  (If I'm wrong here, please
somebody correct me.)

So you can't usefully set
bibtex-entry-offset/bibtex-contline-indentation as file-local variables
without also setting fill-prefix that way, I think?

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





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation
  2020-11-14 16:20 ` Lars Ingebrigtsen
@ 2020-11-16 23:31   ` Roland Winkler
  2020-11-16 23:36     ` Lars Ingebrigtsen
  2020-12-03 19:39     ` Francesco Potortì
  0 siblings, 2 replies; 6+ messages in thread
From: Roland Winkler @ 2020-11-16 23:31 UTC (permalink / raw)
  To: Francesco Potort�=AC; +Cc: Lars Ingebrigtsen, 44618

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 741 bytes --]

On Sat Nov 14 2020 Lars Ingebrigtsen wrote:
> > -  (let ((end-field (copy-marker (bibtex-end-of-field bounds))))
> > +  (let ((fill-prefix
> > +	 (make-string (+ bibtex-entry-offset bibtex-contline-indentation) ?\s))
> > +	(end-field (copy-marker (bibtex-end-of-field bounds))))
> 
> This patch would make it impossible for people to alter fill-prefix
> (from the mode hook, for instance), so I don't think this is the right
> solution.

Can you please try the attached patch using hack-local-variables-hook?
This was 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.


[-- Attachment #2: bibtex.diff --]
[-- Type: application/octet-stream, Size: 3715 bytes --]

--- bibtex.el~	2020-11-15 21:13:56.440216563 -0600
+++ bibtex.el	2020-11-15 21:28:47.313180062 -0600
@@ -890,7 +890,8 @@
 (defcustom bibtex-comment-start "@Comment"
   "String starting a BibTeX comment."
   :group 'bibtex
-  :type 'string)
+  :type 'string
+  :safe #'stringp)
 
 (defcustom bibtex-add-entry-hook nil
   "List of functions to call when BibTeX entry has been inserted."
@@ -1219,7 +1220,8 @@
   "Offset for BibTeX entries.
 Added to the value of all other variables which determine columns."
   :group 'bibtex
-  :type 'integer)
+  :type 'integer
+  :safe #'integerp)
 
 (defcustom bibtex-field-indentation 2
   "Starting column for the name part in BibTeX fields."
@@ -1232,13 +1234,15 @@
   "Starting column for the text part in BibTeX fields.
 Should be equal to the space needed for the longest name part."
   :group 'bibtex
-  :type 'integer)
+  :type 'integer
+  :safe #'integerp)
 
 (defcustom bibtex-contline-indentation
   (+ bibtex-text-indentation 1)
   "Starting column for continuation lines of BibTeX fields."
   :group 'bibtex
-  :type 'integer)
+  :type 'integer
+  :safe #'integerp)
 
 (defcustom bibtex-align-at-equal-sign nil
   "If non-nil, align fields at equal sign instead of field text.
@@ -2941,7 +2945,7 @@
                                          (1+ (match-beginning 3)) (1- (match-end 3)))))
                                (unless (assoc key crossref-keys)
                                  (push (list key) crossref-keys))))
-                            ;; We have probably have a non-bibtex file.
+                            ;; We probably have a non-bibtex file.
                             ((not (match-beginning bibtex-type-in-head))
                              (throw 'userkey nil))
                             ;; only keys of known entries
@@ -3435,15 +3439,10 @@
                                    bibtex-parse-keys-timeout t
                                    'bibtex-parse-buffers-stealthily)))
   (set (make-local-variable 'paragraph-start) "[ \f\n\t]*$")
-  (set (make-local-variable 'comment-start) bibtex-comment-start)
-  (set (make-local-variable 'comment-start-skip)
-       (concat (regexp-quote bibtex-comment-start) "\\>[ \t]*"))
   (set (make-local-variable 'comment-column) 0)
   (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 ((?$ . "\"")
@@ -3462,9 +3461,18 @@
   (set (make-local-variable 'syntax-propertize-function)
        (syntax-propertize-via-font-lock
         bibtex-font-lock-syntactic-keywords))
-  (bibtex-set-dialect nil t)
-  ;; Allow `bibtex-dialect' as a file-local variable.
-  (add-hook 'hack-local-variables-hook #'bibtex-set-dialect nil t))
+  (let ((fun (lambda ()
+               (bibtex-set-dialect)
+               (set (make-local-variable 'comment-start) bibtex-comment-start)
+               (set (make-local-variable 'comment-start-skip)
+                    (concat (regexp-quote bibtex-comment-start) "\\>[ \t]*"))
+               (set (make-local-variable 'fill-prefix)
+                    (make-string (+ bibtex-entry-offset
+                                    bibtex-contline-indentation)
+                                 ?\s)))))
+    (if buffer-file-name
+        (add-hook 'hack-local-variables-hook fun nil t)
+      (funcall fun))))
 
 (defun bibtex-entry-alist (dialect)
   "Return entry-alist for DIALECT."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation
  2020-11-16 23:31   ` Roland Winkler
@ 2020-11-16 23:36     ` Lars Ingebrigtsen
  2020-12-02 18:38       ` Roland Winkler
  2020-12-03 19:39     ` Francesco Potortì
  1 sibling, 1 reply; 6+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-16 23:36 UTC (permalink / raw)
  To: Roland Winkler; +Cc: 44618

"Roland Winkler" <winkler@gnu.org> writes:

> Can you please try the attached patch using hack-local-variables-hook?
> This was 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.

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.)

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





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation
  2020-11-16 23:36     ` Lars Ingebrigtsen
@ 2020-12-02 18:38       ` Roland Winkler
  0 siblings, 0 replies; 6+ messages in thread
From: Roland Winkler @ 2020-12-02 18:38 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Teemu Likonen, 44647-done, 44618-done

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.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation
  2020-11-16 23:31   ` Roland Winkler
  2020-11-16 23:36     ` Lars Ingebrigtsen
@ 2020-12-03 19:39     ` Francesco Potortì
  1 sibling, 0 replies; 6+ messages in thread
From: Francesco Potortì @ 2020-12-03 19:39 UTC (permalink / raw)
  To: Roland Winkler; +Cc: Lars Ingebrigtsen, 44618

>Can you please try the attached patch using hack-local-variables-hook?
>This was partly inspired by the related comment
>
>https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21764#15

Sorry for the delay, I have now managed to test the latest version of
bibtex.el which includes your patch, and it works for me.  I made
various tests and I saw no problems.

Thanks

--
fp





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-12-03 19:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 12:45 bug#44618: 26.3; bibtex.el ignores file variable bibtex-contline-indentation Francesco Potortì
2020-11-14 16:20 ` Lars Ingebrigtsen
2020-11-16 23:31   ` Roland Winkler
2020-11-16 23:36     ` Lars Ingebrigtsen
2020-12-02 18:38       ` Roland Winkler
2020-12-03 19:39     ` Francesco Potortì

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).