bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Rudolf Adamkovič via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Howdy smart folks!

So, today I customized the

> global-display-fill-column-indicator-modes

variable, and it does not work.

I set the variable as per

> Its value is ((not special-mode java-mode swift-mode kotlin-mode) t)
> 
> Original value was
> ((not special-mode)
>  t)

but the fill column indicator shows up in the not-modes listed.

I have also noticed that the help window says

> global-display-fill-column-indicator-modes is a variable defined in
> ‘display-fill-column-indicator.el’.

but the variable is not defined in that source file.

Upon a quick `git grep', it seems that

- NO TESTS for this functionality EXIST, and
- NO SOURCE file in the repository USES the variable.

P.S. The variable is defined only in `ldefs-boot', not
`display-fill-column-indicator'.

Rudy


In GNU Emacs 30.0.50 (build 1, aarch64-apple-darwin22.3.0, NS
 appkit-2299.40 Version 13.2.1 (Build 22D68)) of 2023-03-22 built on
 Rudolfs-MacBook-Air.local
Repository revision: 5056b8e589753698ce7ee935980ca03de0e41bf5
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.2.1

Configured using:
 'configure --with-json --with-rsvg'

Configured features:
ACL GLIB GNUTLS JSON LCMS2 LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER PNG
RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Magit

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  server-mode: t
  corfu-history-mode: t
  global-corfu-mode: t
  corfu-mode: t
  vertico-mode: t
  global-hi-lock-mode: t
  hi-lock-mode: t
  global-hl-todo-mode: t
  global-diff-hl-mode: t
  savehist-mode: t
  pixel-scroll-precision-mode: t
  display-time-mode: t
  delete-selection-mode: t
  global-goto-address-mode: t
  goto-address-mode: t
  global-subword-mode: t
  subword-mode: t
  save-place-mode: t
  global-display-fill-column-indicator-mode: t
  global-whitespace-mode: t
  global-display-line-numbers-mode: t
  global-auto-revert-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  context-menu-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/Users/salutis/.emacs.d/elpa/modus-themes-20230320.449/theme-loaddefs hides /Users/salutis/src/emacs/nextstep/Emacs.app/Contents/Resources/lisp/theme-loaddefs
/Users/salutis/.emacs.d/elpa/transient-20230315.1520/transient hides /Users/salutis/src/emacs/nextstep/Emacs.app/Contents/Resources/lisp/transient

Features:
(shadow notmuch notmuch-tree notmuch-jump notmuch-hello notmuch-show
notmuch-print notmuch-crypto notmuch-mua notmuch-message notmuch-draft
notmuch-maildir-fcc notmuch-address notmuch-company notmuch-parser
notmuch-wash coolj icalendar diary-lib diary-loaddefs notmuch-tag
notmuch-lib notmuch-version notmuch-compat mm-view mml-smime smime
gnutls dig tempo mail-extr emacsbug tabify cl-print pulse color files-x
grep dired-aux vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs dired-x
diff-hl-dired shortdoc mode-local vc-git misearch multi-isearch
face-remap magit-extras magit-submodule magit-blame magit-stash
magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone
magit-remote magit-commit magit-sequence magit-notes magit-worktree
magit-tag magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
which-func imenu magit-diff smerge-mode diff git-commit log-edit message
yank-media rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader add-log magit-core magit-autorevert magit-margin
magit-transient magit-process with-editor server magit-mode transient
magit-git magit-base magit-section crm dash char-fold hl-line
network-stream puny nsm emms-player-mpd emms-url tq emms-player-simple
emms-browser sort emms-playlist-sort emms-last-played emms-volume
emms-volume-sndioctl emms-volume-mixerctl emms-volume-pulse
emms-volume-amixer emms-playlist-mode emms-source-playlist
emms-source-file locate ls-lisp dired dired-loaddefs emms-cache
emms-info emms-later-do emms emms-compat paredit edmacro kmacro
corfu-history corfu vertico hi-lock hl-todo compat diff-hl log-view
pcvs-util vc-dir ewoc vc vc-dispatcher diff-mode cus-start orderless
pdf-loader finder-inf ob-sqlite ob-sql ob-lisp ob-scheme geiser-impl
help-fns radix-tree geiser-custom geiser-base geiser savehist ob-R
ob-plantuml ob-org org-clock modus-operandi-theme modus-themes
ob-makefile ob-lua ob-latex ob-java ob-dot slime apropos etags fileloop
generator xref arc-mode archive-mode hyperspec flyspell ispell fortune
flymake-proc flymake project compile text-property-search pixel-scroll
cua-base time comp derived comp-cstr warnings cl-extra help-mode delsel
goto-addr thingatpt cap-words superword subword saveplace easy-mmode
display-fill-column-indicator disp-table whitespace display-line-numbers
autorevert filenotify ob-clojure cus-edit pp cus-load wid-edit ob-C
cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine
cc-vars cc-defs bug-reference ob-shell shell org ob ob-tangle ob-ref
ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
pcomplete comint ansi-osc ansi-color ring org-list org-footnote
org-faces org-entities time-date noutline outline icons ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol rx org-fold org-fold-core
org-keys oc org-loaddefs find-func cal-menu calendar cal-loaddefs
org-version org-compat org-macs format-spec sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils org-drill-autoloads
bbdb-autoloads paredit-autoloads lua-mode-autoloads htmlize-autoloads
yaml-mode-autoloads graphviz-dot-mode-autoloads kotlin-mode-autoloads
corfu-autoloads orderless-autoloads magit-autoloads pcase
magit-section-autoloads geiser-guile-autoloads geiser-autoloads
git-commit-autoloads fennel-mode-autoloads cape-autoloads
cider-autoloads spinner-autoloads flymake-grammarly-autoloads
grammarly-autoloads citar-autoloads citeproc-autoloads queue-autoloads
f-autoloads dash-autoloads parsebib-autoloads bnf-mode-autoloads
emms-autoloads websocket-autoloads rainbow-mode-autoloads
clojure-mode-autoloads vertico-autoloads request-autoloads
telega-autoloads rainbow-identifiers-autoloads sql-indent-autoloads
parseedn-autoloads modus-themes-autoloads string-inflection-autoloads
hl-todo-autoloads pdf-tools-autoloads transient-autoloads
sesman-autoloads visual-fill-column-autoloads tablist-autoloads
mentor-autoloads async-autoloads xml-rpc-autoloads ebnf-mode-autoloads
url-scgi-autoloads elfeed-tube-mpv-autoloads mpv-autoloads
elfeed-tube-autoloads aio-autoloads elfeed-autoloads yasnippet-autoloads
markdown-mode-autoloads slime-autoloads macrostep-autoloads
parseclj-autoloads persist-autoloads chess-autoloads diff-hl-autoloads
sqlup-mode-autoloads ess-autoloads password-store-autoloads
with-editor-autoloads info compat-autoloads s-autoloads
swift-mode-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip
cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads kqueue cocoa ns lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 1056155 112543)
 (symbols 48 41559 29)
 (strings 32 259454 17283)
 (string-bytes 1 7465899)
 (vectors 16 79633)
 (vector-slots 8 998299 124451)
 (floats 8 412 1895)
 (intervals 56 29667 6291)
 (buffers 984 32))

-- 
"The whole science is nothing more than a refinement of everyday
thinking."
-- Albert Einstein, 1879-1955

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Eli Zaretskii]

> Date: Wed, 22 Mar 2023 13:03:13 +0100
> From:  Rudolf Adamkovič via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> So, today I customized the
> 
> > global-display-fill-column-indicator-modes
> 
> variable, and it does not work.
> 
> I set the variable as per
> 
> > Its value is ((not special-mode java-mode swift-mode kotlin-mode) t)
> > 
> > Original value was
> > ((not special-mode)
> >  t)
> 
> but the fill column indicator shows up in the not-modes listed.

How did you customize it?

> I have also noticed that the help window says
> 
> > global-display-fill-column-indicator-modes is a variable defined in
> > ‘display-fill-column-indicator.el’.
> 
> but the variable is not defined in that source file.

Actually, it is: it is defined as a side effect of
define-globalized-minor-mode call.

(But in my case Emacs says the variable is defined in loaddefs.el,
which is not good.)

> Upon a quick `git grep', it seems that
> 
> - NO TESTS for this functionality EXIST, and

The tests seem to be in test/lisp/emacs-lisp/easy-mmode-tests.el, see
the test easy-mmode--globalized-predicate.  And they pass.

Adding Lars, who wrote that code, and Stefan, in case they have
comments or suggestions.

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Rudolf Adamkovič via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> How did you customize it?

The relevant parts (in the order they execute):

  ;; Set the fill column.
  ;;
  (setopt fill-column 80)

  ;; Define the "not" modes.
  ;;
  (defconst not-modes
    '(special-mode
      java-mode swift-mode kotlin-mode
      telega-chat-mode))

  ;; Highlight text beyond 80 columns (and also tabs).
  ;;
  ;; THIS (SIMILAR) PART WORKS
  ;;
  (setopt whitespace-line-column 80
          whitespace-style '(face tabs lines-tail)
          whitespace-global-modes (cons 'not not-modes))
  (global-whitespace-mode)

  ;; Show a vertical line at 80 columns.
  ;;
  ;; THIS PART DOES NOT WORK
  ;;
  (setopt display-fill-column-indicator-column 80)
  (setopt global-display-fill-column-indicator-modes
          (list (append '(not) not-modes) t))
  (global-display-fill-column-indicator-mode)

> Actually, it is [...] And they pass.

I missed *all* that; thank you for letting me know!

Rudy
-- 
"Genius is 1% inspiration and 99% perspiration."
-- Thomas Alva Edison, 1932

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> So, today I customized the
>> 
>> > global-display-fill-column-indicator-modes
>> 
>> variable, and it does not work.

I think it's a thinko in the code: we define the variable but the code
hardcode the var's default value instead of looking up the variable's
actual value.

The patch below should fix it.


        Stefan


diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 5721470ad0d..e84207da2df 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -509,7 +509,7 @@ define-globalized-minor-mode
          (setq turn-on-function
                `(lambda ()
                   (require 'easy-mmode)
-                  (when (easy-mmode--globalized-predicate-p ,(car predicate))
+                  (when (easy-mmode--globalized-predicate-p ,MODE-predicate)
                     (funcall ,turn-on-function)))))
         (_ (push keyw extra-keywords) (push (pop body) extra-keywords))))
 

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Rudolf Adamkovič <salutis@me.com>,  Lars Ingebrigtsen
>  <larsi@gnus.org>,
>   62376@debbugs.gnu.org
> Date: Wed, 22 Mar 2023 13:52:50 -0400
> 
> >> So, today I customized the
> >> 
> >> > global-display-fill-column-indicator-modes
> >> 
> >> variable, and it does not work.
> 
> I think it's a thinko in the code: we define the variable but the code
> hardcode the var's default value instead of looking up the variable's
> actual value.
> 
> The patch below should fix it.

Thanks, I verified that this fixes the bug, and installed it on the
emacs-29 branch.

Closing.

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Eli Zaretskii]

> Cc: larsi@gnus.org, salutis@me.com, 62376-done@debbugs.gnu.org
> Date: Thu, 23 Mar 2023 11:04:48 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: Rudolf Adamkovič <salutis@me.com>,  Lars Ingebrigtsen
> >  <larsi@gnus.org>,
> >   62376@debbugs.gnu.org
> > Date: Wed, 22 Mar 2023 13:52:50 -0400
> > 
> > >> So, today I customized the
> > >> 
> > >> > global-display-fill-column-indicator-modes
> > >> 
> > >> variable, and it does not work.
> > 
> > I think it's a thinko in the code: we define the variable but the code
> > hardcode the var's default value instead of looking up the variable's
> > actual value.
> > 
> > The patch below should fix it.
> 
> Thanks, I verified that this fixes the bug, and installed it on the
> emacs-29 branch.

Hmm... but "C-h v" still tells me the variable is defined in
loaddefs.el, until display-fill-column-indicator.el is loaded.  Can we
do anything to prevent this?

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Thanks, I verified that this fixes the bug, and installed it on the
>> emacs-29 branch.
> Hmm... but "C-h v" still tells me the variable is defined in
> loaddefs.el, until display-fill-column-indicator.el is loaded.

I think it's an unrelated problem.

> Can we do anything to prevent this?

Probably.  The code that "fixes/adjusts" this is an ugly hack I came up
with many years ago and it's quite brittle.  My guess is that it doesn't
work any more because of some change to the way we build/handle
`loaddefs.el` (the code works by using `load-history` to see the
"origin" of the variable, (in the present case it should say
`loaddefs.el(c)`) then look for the actual place in that file
where the variable is found, and then checks to see if that place looks
like those produced by `autoload.el` (which here should be the case)
and then uses the comments added by `autoload.el` to find the *real*
origin).

We should really replace it with something more explicit/reliable so we
don't have to "parse" the `loaddefs.el` file, but it's definitely out of
the scope of `emacs-29`.

In the mean time, the patch below seems to make the ugly hack work again
it for me.


        Stefan


diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index f9edf14a1da..b040ec49214 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -438,7 +438,7 @@ find-lisp-object-file-name
       (setq file-name
 	    (locate-file file-name load-path '(".el" ".elc") 'readable)))
      ((and (stringp file-name)
-	   (string-match "[.]*loaddefs.el\\'" file-name))
+	   (string-match "[.]*loaddefs.elc?\\'" file-name))
       ;; An autoloaded variable or face.  Visit loaddefs.el in a buffer
       ;; and try to extract the defining file.  The following form is
       ;; from `describe-function-1' and `describe-variable'.

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 62376@debbugs.gnu.org,  salutis@me.com
> Date: Thu, 23 Mar 2023 09:16:51 -0400
> 
> We should really replace it with something more explicit/reliable so we
> don't have to "parse" the `loaddefs.el` file, but it's definitely out of
> the scope of `emacs-29`.
> 
> In the mean time, the patch below seems to make the ugly hack work again
> it for me.

Thanks, please install on emacs-29.

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Thanks, please install on emacs-29.

Done, closing,


        Stefan

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Rudolf Adamkovič via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Thanks, please install on emacs-29.
>
> Done, closing,

Where was this fix installed?

I cannot find the commit on `emacs-29' (nor `master').

Rudy
-- 
"The whole science is nothing more than a refinement of everyday
thinking."
-- Albert Einstein, 1879-1955

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Rudolf Adamkovič [2023-03-27 11:34:02] wrote:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> Thanks, please install on emacs-29.
>>
>> Done, closing,
>
> Where was this fix installed?

On `master` in commit 5904bdf581974cb8bf88534bbc58b13cca797303.
Hmm... wait you're talking about the one... indeed it's not there...
Let me check what's going on and it will hopefully appear on
`master` shortly.


        Stefan

bug#62376: 30.0.50; global-display-fill-column-indicator-modes broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> On `master` in commit 5904bdf581974cb8bf88534bbc58b13cca797303.
> Hmm... wait you're talking about the one... indeed it's not there...
> Let me check what's going on and it will hopefully appear on
> `master` shortly.

Oh the other one was safer so it went to `emacs-29` as commit
d93a439846f03dfb2be28d6b5c2e963ef6be0c22.
I just merged `emacs-29` into `master` so I think it should be all on
`master` now.


        Stefan