unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
@ 2020-05-09  8:30 Philipp Stephani
  2020-05-09  9:37 ` Eli Zaretskii
  2020-08-28 14:48 ` Mauro Aranda
  0 siblings, 2 replies; 20+ messages in thread
From: Philipp Stephani @ 2020-05-09  8:30 UTC (permalink / raw)
  To: 41145


M-x customize-group RET display-fill-column-indicator RET, then expand
the section for `display-fill-column-indicator-character'.  The
customization buffer then contains:

Display Fill Column Indicator group:  Group definition missing. 
       State : visible group members are all at standard values.
   
[...]

Hide display-fill-column-indicator-character: nil
    State : STANDARD. (mismatch)
   Character to draw the indicator when ‘display-fill-column-indicator’ is non-nil. More

The two issues here are:

1. "Group definition missing."  It looks like customizing this group
   should load `display-fill-column-indicator-mode', which defines this
   group, or the group definition should be in cus-start.el.

2. "(mismatch)."  The default value for
   `display-fill-column-indicator-character' is nil, but its type is
   `character', so nil isn't allowed.


In GNU Emacs 27.0.91 (build 9, x86_64-pc-linux-gnu, GTK+ Version 3.24.13)
 of 2020-05-09
Repository revision: d5c184aa3e2a183144efa5f269e2c70d2681aa0a
Repository branch: emacs-27
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux rodete

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --enable-checking=all --enable-gtk-deprecation-warnings
 --enable-gcc-warnings=warn-only --enable-check-lisp-object-type
 --with-mailutils --without-pop 'CFLAGS=-O0 -g3' LDFLAGS=-g3'

Configured features:
XPM JPEG TIFF GIF PNG SOUND DBUS GSETTINGS GLIB NOTIFY INOTIFY
LIBSELINUX GNUTLS FREETYPE HARFBUZZ XFT ZLIB TOOLKIT_SCROLL_BARS GTK3
X11 XDBE XIM MODULES THREADS PDUMPER GMP

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec epa epg epg-config gnus-util
rmail rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils phst skeleton
derived edmacro kmacro pcase ffap thingatpt url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json map url-vars mailcap subr-x rx gnutls puny seq
byte-opt gv bytecomp byte-compile cconv dbus xml cl-loaddefs cl-lib
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic 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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 63403 5547)
 (symbols 48 8443 1)
 (strings 32 22409 1678)
 (string-bytes 1 711978)
 (vectors 16 12504)
 (vector-slots 8 174118 5994)
 (floats 8 25 34)
 (intervals 56 201 0)
 (buffers 1000 12))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

If you received this communication by mistake, please don’t forward it to
anyone else (it may contain confidential or privileged information), please
erase all copies of it, including all attachments, and please let the sender
know it went to the wrong person.  Thanks.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-05-09  8:30 bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group Philipp Stephani
@ 2020-05-09  9:37 ` Eli Zaretskii
  2020-05-09 12:31   ` Philipp Stephani
  2020-08-28 14:48 ` Mauro Aranda
  1 sibling, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2020-05-09  9:37 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 41145

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 09 May 2020 10:30:13 +0200
> 
> The two issues here are:
> 
> 1. "Group definition missing."  It looks like customizing this group
>    should load `display-fill-column-indicator-mode', which defines this
>    group, or the group definition should be in cus-start.el.
The same seems to work for display-line-numbers, and I don't think I
see the crucial difference.

> 2. "(mismatch)."  The default value for
>    `display-fill-column-indicator-character' is nil, but its type is
>    `character', so nil isn't allowed.

I fixed this, thanks.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-05-09  9:37 ` Eli Zaretskii
@ 2020-05-09 12:31   ` Philipp Stephani
  0 siblings, 0 replies; 20+ messages in thread
From: Philipp Stephani @ 2020-05-09 12:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41145

Am Sa., 9. Mai 2020 um 11:37 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 09 May 2020 10:30:13 +0200
> >
> > The two issues here are:
> >
> > 1. "Group definition missing."  It looks like customizing this group
> >    should load `display-fill-column-indicator-mode', which defines this
> >    group, or the group definition should be in cus-start.el.
> The same seems to work for display-line-numbers, and I don't think I
> see the crucial difference.

My guess is that something is wrong with cus-dep.el, because the f-c-i
group doesn't appear in cus-load.el.

>
> > 2. "(mismatch)."  The default value for
> >    `display-fill-column-indicator-character' is nil, but its type is
> >    `character', so nil isn't allowed.
>
> I fixed this, thanks.

Thanks





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-05-09  8:30 bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group Philipp Stephani
  2020-05-09  9:37 ` Eli Zaretskii
@ 2020-08-28 14:48 ` Mauro Aranda
  2020-08-29  6:54   ` Eli Zaretskii
  1 sibling, 1 reply; 20+ messages in thread
From: Mauro Aranda @ 2020-08-28 14:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41145, Philipp Stephani

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philipp Stephani <p.stephani2@gmail.com>
>> Date: Sat, 09 May 2020 10:30:13 +0200
>>
>> The two issues here are:
>>
>> 1. "Group definition missing."  It looks like customizing this group
>>    should load `display-fill-column-indicator-mode', which defines this
>>    group, or the group definition should be in cus-start.el.
> The same seems to work for display-line-numbers, and I don't think I
> see the crucial difference.

I did some debugging and found that the difference is that there are no
custom options or faces defined in display-fill-column-indicator.el:
fill-column-indicator is in faces.el and
display-fill-column-indicator, display-fill-column-indicator-character
and display-fill-column-indicator-column are in xdisp.c

custom-make-dependencies doesn't go through faces.el, since it's
preloaded, so it doesn't record the custom-where property into
fill-column-indicator.
And since cus-dep.el doesn't require cus-start, fill-column-indicator is
the only member of the custom-group property of
display-fill-column-indicator, so there is no way
custom-make-dependencies will record the custom-where property into any
of the options defined in xdisp.c.

So later on, when looking for a custom-where property in the members of
the display-fill-column-indicator group, we find nothing, resulting in
no custom-loads thingy for display-fill-column-indicator.

The problem goes away if we require cus-start in cus-dep, or if we
record into the custom-loads the file where to find the custom-group.
That is, add in the first mapatoms call something like this:
(when (get symbol 'custom-where)
  (push (get symbol 'custom-where) found))

Or if eventually a defface or a defcustom makes its way into
display-fill-column-indicator.el, of course.

Note that adding the above form will result in adding cus-edit to the
custom-loads property of the groups defined in cus-edit (like editing,
convenience, etc.).  I don't know if that is harmless.

[-- Attachment #2: Type: text/html, Size: 2349 bytes --]

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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-28 14:48 ` Mauro Aranda
@ 2020-08-29  6:54   ` Eli Zaretskii
  2020-08-29 15:36     ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2020-08-29  6:54 UTC (permalink / raw)
  To: Mauro Aranda, Stefan Monnier; +Cc: 41145, p.stephani2

> From: Mauro Aranda <maurooaranda@gmail.com>
> Date: Fri, 28 Aug 2020 11:48:57 -0300
> Cc: Philipp Stephani <p.stephani2@gmail.com>, 41145@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philipp Stephani <p.stephani2@gmail.com>
> >> Date: Sat, 09 May 2020 10:30:13 +0200
> >>
> >> The two issues here are:
> >>
> >> 1. "Group definition missing."  It looks like customizing this group
> >>    should load `display-fill-column-indicator-mode', which defines this
> >>    group, or the group definition should be in cus-start.el.
> > The same seems to work for display-line-numbers, and I don't think I
> > see the crucial difference.
> 
> I did some debugging and found that the difference is that there are no
> custom options or faces defined in display-fill-column-indicator.el:
> fill-column-indicator is in faces.el and
> display-fill-column-indicator, display-fill-column-indicator-character
> and display-fill-column-indicator-column are in xdisp.c
> 
> custom-make-dependencies doesn't go through faces.el, since it's
> preloaded, so it doesn't record the custom-where property into
> fill-column-indicator.
> And since cus-dep.el doesn't require cus-start, fill-column-indicator is
> the only member of the custom-group property of
> display-fill-column-indicator, so there is no way
> custom-make-dependencies will record the custom-where property into any
> of the options defined in xdisp.c.
> 
> So later on, when looking for a custom-where property in the members of
> the display-fill-column-indicator group, we find nothing, resulting in
> no custom-loads thingy for display-fill-column-indicator.
> 
> The problem goes away if we require cus-start in cus-dep, or if we
> record into the custom-loads the file where to find the custom-group.
> That is, add in the first mapatoms call something like this:
> (when (get symbol 'custom-where)
>   (push (get symbol 'custom-where) found))
> 
> Or if eventually a defface or a defcustom makes its way into
> display-fill-column-indicator.el, of course.
> 
> Note that adding the above form will result in adding cus-edit to the
> custom-loads property of the groups defined in cus-edit (like editing,
> convenience, etc.).  I don't know if that is harmless.

Stefan, any suggestions regarding which solution route is preferable?

Thanks.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-29  6:54   ` Eli Zaretskii
@ 2020-08-29 15:36     ` Stefan Monnier
  2020-08-29 16:23       ` Mauro Aranda
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2020-08-29 15:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41145, p.stephani2, Mauro Aranda

>> I did some debugging and found that the difference is that there are no
>> custom options or faces defined in display-fill-column-indicator.el:
>> fill-column-indicator is in faces.el and
>> display-fill-column-indicator, display-fill-column-indicator-character
>> and display-fill-column-indicator-column are in xdisp.c

Which begs the question: why define the group there?

>> Or if eventually a defface or a defcustom makes its way into
>> display-fill-column-indicator.el, of course.

Actually, there is a defcustom there (in the expansion of
`define-globalized-minor-mode`).

>> Note that adding the above form will result in adding cus-edit to the
>> custom-loads property of the groups defined in cus-edit (like editing,
>> convenience, etc.).  I don't know if that is harmless.
>
> Stefan, any suggestions regarding which solution route is preferable?

I'd get rid of the file and move its content to one of the
preloaded files (simple.el?  faces.el?  fill.el? ...)


        Stefan






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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-29 15:36     ` Stefan Monnier
@ 2020-08-29 16:23       ` Mauro Aranda
  2020-08-30  3:58         ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Mauro Aranda @ 2020-08-29 16:23 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, Philipp Stephani

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

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

>>> Or if eventually a defface or a defcustom makes its way into
>>> display-fill-column-indicator.el, of course.
>
> Actually, there is a defcustom there (in the expansion of
> `define-globalized-minor-mode`).

Sorry, I wasn't clear enough.  There is no defcustom or defface whose
:group is display-fill-column-indicator.

[-- Attachment #2: Type: text/html, Size: 528 bytes --]

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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-29 16:23       ` Mauro Aranda
@ 2020-08-30  3:58         ` Stefan Monnier
  2020-08-30 11:52           ` Mauro Aranda
  2020-08-30 13:50           ` Eli Zaretskii
  0 siblings, 2 replies; 20+ messages in thread
From: Stefan Monnier @ 2020-08-30  3:58 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 41145, Philipp Stephani

>>>> Or if eventually a defface or a defcustom makes its way into
>>>> display-fill-column-indicator.el, of course.
>>
>> Actually, there is a defcustom there (in the expansion of
>> `define-globalized-minor-mode`).
>
> Sorry, I wasn't clear enough.  There is no defcustom or defface whose
> :group is display-fill-column-indicator.

`global-display-fill-column-indicator-mode` should definitely belong to
the `display-fill-column-indicator` group (because the `defcustom`
doesn't have an explicit `:group` so it should fallback to using the
last-defined group).

BTW, another quick-fix might be to move the defface definition to
display-fill-column-indicator.el.


        Stefan






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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30  3:58         ` Stefan Monnier
@ 2020-08-30 11:52           ` Mauro Aranda
  2020-08-30 14:51             ` Stefan Monnier
  2020-08-30 13:50           ` Eli Zaretskii
  1 sibling, 1 reply; 20+ messages in thread
From: Mauro Aranda @ 2020-08-30 11:52 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, Philipp Stephani

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

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

>>>>> Or if eventually a defface or a defcustom makes its way into
>>>>> display-fill-column-indicator.el, of course.
>>>
>>> Actually, there is a defcustom there (in the expansion of
>>> `define-globalized-minor-mode`).
>>
>> Sorry, I wasn't clear enough.  There is no defcustom or defface whose
>> :group is display-fill-column-indicator.
>
> `global-display-fill-column-indicator-mode` should definitely belong to
> the `display-fill-column-indicator` group (because the `defcustom`
> doesn't have an explicit `:group` so it should fallback to using the
> last-defined group).

Is it, though?
emacs -Q
M-x customize-option RET global-display-fill-column-indicator-mode
And near the end of buffer I read:
Groups: Global Display Fill Column Indicator

Furthermore, the list
(global-display-fill-column-indicator-mode custom-variable) is not a
member of the custom-group property of display-fill-column-indicator.

> BTW, another quick-fix might be to move the defface definition to
> display-fill-column-indicator.el.

Right.

[-- Attachment #2: Type: text/html, Size: 1344 bytes --]

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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30  3:58         ` Stefan Monnier
  2020-08-30 11:52           ` Mauro Aranda
@ 2020-08-30 13:50           ` Eli Zaretskii
  1 sibling, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2020-08-30 13:50 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, p.stephani2, maurooaranda

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Philipp Stephani <p.stephani2@gmail.com>,
>   41145@debbugs.gnu.org
> Date: Sat, 29 Aug 2020 23:58:04 -0400
> 
> >>>> Or if eventually a defface or a defcustom makes its way into
> >>>> display-fill-column-indicator.el, of course.
> >>
> >> Actually, there is a defcustom there (in the expansion of
> >> `define-globalized-minor-mode`).
> >
> > Sorry, I wasn't clear enough.  There is no defcustom or defface whose
> > :group is display-fill-column-indicator.
> 
> `global-display-fill-column-indicator-mode` should definitely belong to
> the `display-fill-column-indicator` group (because the `defcustom`
> doesn't have an explicit `:group` so it should fallback to using the
> last-defined group).

So you are saying this should have worked as it is?

> BTW, another quick-fix might be to move the defface definition to
> display-fill-column-indicator.el.

The idea was not to require display-fill-column-indicator.el be loaded
to activate the feature.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30 11:52           ` Mauro Aranda
@ 2020-08-30 14:51             ` Stefan Monnier
  2020-08-30 15:09               ` Eli Zaretskii
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Stefan Monnier @ 2020-08-30 14:51 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 41145, Philipp Stephani

>> `global-display-fill-column-indicator-mode` should definitely belong to
>> the `display-fill-column-indicator` group (because the `defcustom`
>> doesn't have an explicit `:group` so it should fallback to using the
>> last-defined group).
>
> Is it, though?
> emacs -Q
> M-x customize-option RET global-display-fill-column-indicator-mode
> And near the end of buffer I read:
> Groups: Global Display Fill Column Indicator

Oh, indeed, it's an old misfeature that was introduced by our
idiot-in-chief (tho to my defense, I think it made some sense back then
because `defcustom` did not have a useful default for `:group`).

We can fix it either by adding `:group 'display-fill-column-indicator`
to the `define-globalized-minor-mode` or by getting rid of the
misfeature, as in the patch below (this patch will likely fix a few
other similar cases).

Eli?


        Stefan


diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 24c9e79f2c..e3eb9294ed 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -157,9 +157,6 @@ define-minor-mode
   the minor mode is global):
 
 :group GROUP	Custom group name to use in all generated `defcustom' forms.
-		Defaults to MODE without the possible trailing \"-mode\".
-		Don't use this default group name unless you have written a
-		`defgroup' to define that group properly.
 :global GLOBAL	If non-nil specifies that the minor mode is not meant to be
 		buffer-local, so don't make the variable MODE buffer-local.
 		By default, the mode is buffer-local.
@@ -262,12 +259,6 @@ define-minor-mode
     (unless initialize
       (setq initialize '(:initialize 'custom-initialize-default)))
 
-    (unless group
-      ;; We might as well provide a best-guess default group.
-      (setq group
-	    `(:group ',(intern (replace-regexp-in-string
-				"-mode\\'" "" mode-name)))))
-
     ;; TODO? Mark booleans as safe if booleanp?  Eg abbrev-mode.
     (unless type (setq type '(:type 'boolean)))
 






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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30 14:51             ` Stefan Monnier
@ 2020-08-30 15:09               ` Eli Zaretskii
  2020-08-30 17:03                 ` Stefan Monnier
  2020-08-30 15:09               ` Drew Adams
  2020-09-11 23:26               ` Michael Heerdegen
  2 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2020-08-30 15:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, p.stephani2, maurooaranda

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Philipp Stephani <p.stephani2@gmail.com>,
>   41145@debbugs.gnu.org
> Date: Sun, 30 Aug 2020 10:51:23 -0400
> 
> > emacs -Q
> > M-x customize-option RET global-display-fill-column-indicator-mode
> > And near the end of buffer I read:
> > Groups: Global Display Fill Column Indicator
> 
> Oh, indeed, it's an old misfeature that was introduced by our
> idiot-in-chief (tho to my defense, I think it made some sense back then
> because `defcustom` did not have a useful default for `:group`).
> 
> We can fix it either by adding `:group 'display-fill-column-indicator`
> to the `define-globalized-minor-mode` or by getting rid of the
> misfeature, as in the patch below (this patch will likely fix a few
> other similar cases).
> 
> Eli?

How about doing the former on emacs-27, and the latter on master?

Thanks.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30 14:51             ` Stefan Monnier
  2020-08-30 15:09               ` Eli Zaretskii
@ 2020-08-30 15:09               ` Drew Adams
  2020-09-11 23:26               ` Michael Heerdegen
  2 siblings, 0 replies; 20+ messages in thread
From: Drew Adams @ 2020-08-30 15:09 UTC (permalink / raw)
  To: Stefan Monnier, Mauro Aranda; +Cc: 41145, Philipp Stephani

FWIW, I've said it before, and reiterate now: I
think :group should always be explicitly present.

We've gone to the other extreme, of not only
encouraging its removal (apart from an initial
occurrence), but even removing it from patches
that contained it.

Why do I feel this way?  Repeating what I've
said before:

1. Nothing is really gained.  The amount of
   additional text eliminated is trivial - tiny.

2. A defcustom that's not near a previous one
   has its :group unclear (for humans).

3. It's problematic and error prone wrt moving
   defcustoms around.

It doesn't hurt, and it can help human readers
to just include explicit :group.

Whether that should just be conventional (i.e.,
recommended/encouraged) or actually enforced is
a different question.  I'm not sure it needs to
be enforced (e.g. warning or error msg), but I
don't think that would hurt, and it might help.

Just one opinion.  I repeat it here only because
I haven't mentioned it in a long time - probably
not since the crusade to remove "unnecessary"
:groups was initiated.

(And no, this is not very important.  Just sayin.)





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30 15:09               ` Eli Zaretskii
@ 2020-08-30 17:03                 ` Stefan Monnier
  2020-09-04  2:24                   ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2020-08-30 17:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41145, p.stephani2, maurooaranda

>> > emacs -Q
>> > M-x customize-option RET global-display-fill-column-indicator-mode
>> > And near the end of buffer I read:
>> > Groups: Global Display Fill Column Indicator
>> 
>> Oh, indeed, it's an old misfeature that was introduced by our
>> idiot-in-chief (tho to my defense, I think it made some sense back then
>> because `defcustom` did not have a useful default for `:group`).
>> 
>> We can fix it either by adding `:group 'display-fill-column-indicator`
>> to the `define-globalized-minor-mode` or by getting rid of the
>> misfeature, as in the patch below (this patch will likely fix a few
>> other similar cases).
>> 
>> Eli?
>
> How about doing the former on emacs-27, and the latter on master?

Oh, indeed, I did not intend to change define-minor-mode on the
emacs-27 branch.
So, we have a deal!


        Stefan






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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30 17:03                 ` Stefan Monnier
@ 2020-09-04  2:24                   ` Stefan Monnier
  2020-09-04  7:09                     ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2020-09-04  2:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41145, p.stephani2, maurooaranda

>> How about doing the former on emacs-27, and the latter on master?
> Oh, indeed, I did not intend to change define-minor-mode on the
> emacs-27 branch.
> So, we have a deal!

O, so I looked into fixing it on `emacs-27` and it's not quite as
straightforward as I thought.  Here's the patch that I have currently,
because without that `cus-dep.el` patch, the
`define-globalized-minor-mode` is just completely ignored by
cus-dep anyway.

I think it's quite safe, but I'd like to make sure you agree this is
acceptable for `emacs-27`.


        Stefan


diff --git a/lisp/cus-dep.el b/lisp/cus-dep.el
index fd307a5c04..e2fd7febd2 100644
--- a/lisp/cus-dep.el
+++ b/lisp/cus-dep.el
@@ -99,7 +99,7 @@ custom-make-dependencies
                   (setq name (intern name)))
               (condition-case nil
                   (while (re-search-forward
-                          "^(def\\(custom\\|face\\|group\\)" nil t)
+                          "^(def\\(custom\\|face\\|group\\|ine\\(?:-globalized\\)?-minor-mode\\)" nil t)
                     (beginning-of-line)
                     (let ((type (match-string 1))
 			  (expr (read (current-buffer))))
diff --git a/lisp/display-fill-column-indicator.el b/lisp/display-fill-column-indicator.el
index 3f947bdc1c..3391aa371b 100644
--- a/lisp/display-fill-column-indicator.el
+++ b/lisp/display-fill-column-indicator.el
@@ -73,7 +73,9 @@ display-fill-column-indicator--turn-on
 
 ;;;###autoload
 (define-globalized-minor-mode global-display-fill-column-indicator-mode
-  display-fill-column-indicator-mode display-fill-column-indicator--turn-on)
+  display-fill-column-indicator-mode display-fill-column-indicator--turn-on
+  ;; See bug#41145
+  :group 'display-fill-column-indicator)
 
 (provide 'display-fill-column-indicator)
 









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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-09-04  2:24                   ` Stefan Monnier
@ 2020-09-04  7:09                     ` Eli Zaretskii
  2020-09-04 12:47                       ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2020-09-04  7:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, p.stephani2, maurooaranda

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 41145@debbugs.gnu.org,  p.stephani2@gmail.com,  maurooaranda@gmail.com
> Date: Thu, 03 Sep 2020 22:24:14 -0400
> 
> >> How about doing the former on emacs-27, and the latter on master?
> > Oh, indeed, I did not intend to change define-minor-mode on the
> > emacs-27 branch.
> > So, we have a deal!
> 
> O, so I looked into fixing it on `emacs-27` and it's not quite as
> straightforward as I thought.  Here's the patch that I have currently,
> because without that `cus-dep.el` patch, the
> `define-globalized-minor-mode` is just completely ignored by
> cus-dep anyway.
> 
> I think it's quite safe, but I'd like to make sure you agree this is
> acceptable for `emacs-27`.

Yes, LGTM.  Thanks.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-09-04  7:09                     ` Eli Zaretskii
@ 2020-09-04 12:47                       ` Stefan Monnier
  0 siblings, 0 replies; 20+ messages in thread
From: Stefan Monnier @ 2020-09-04 12:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41145-done, p.stephani2, maurooaranda

>> I think it's quite safe, but I'd like to make sure you agree this is
>> acceptable for `emacs-27`.
> Yes, LGTM.  Thanks.

OK, pushed,


        Stefan






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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-08-30 14:51             ` Stefan Monnier
  2020-08-30 15:09               ` Eli Zaretskii
  2020-08-30 15:09               ` Drew Adams
@ 2020-09-11 23:26               ` Michael Heerdegen
  2020-09-12 15:43                 ` Stefan Monnier
  2 siblings, 1 reply; 20+ messages in thread
From: Michael Heerdegen @ 2020-09-11 23:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, Philipp Stephani, Mauro Aranda

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

> -    (unless group
> -      ;; We might as well provide a best-guess default group.
> -      (setq group
> -	    `(:group ',(intern (replace-regexp-in-string
> -				"-mode\\'" "" mode-name)))))
> -

After this change I have to specify a group for every define-minor-mode
in my init file, otherwise I'm flooded with warnings.  It's a bit
annoying.  I don't care about groups there since I don't use customize
myself at all.  What's the suggestion to handle this use case?

Michael.





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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-09-11 23:26               ` Michael Heerdegen
@ 2020-09-12 15:43                 ` Stefan Monnier
  2020-09-12 18:51                   ` Michael Heerdegen
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2020-09-12 15:43 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 41145, Philipp Stephani, Mauro Aranda

>> -    (unless group
>> -      ;; We might as well provide a best-guess default group.
>> -      (setq group
>> -	    `(:group ',(intern (replace-regexp-in-string
>> -				"-mode\\'" "" mode-name)))))
>
> After this change I have to specify a group for every define-minor-mode
> in my init file, otherwise I'm flooded with warnings.

Hmm... so you have `define-minor-mode`s in your init file?  That sounds
rather unusual [ And "flooded" suggests you have very many.  ]

> It's a bit annoying.  I don't care about groups there since I don't
> use customize myself at all.  What's the suggestion to handle this
> use case?

Add a dummy `defgroup` at the beginning of the file?


        Stefan






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

* bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group
  2020-09-12 15:43                 ` Stefan Monnier
@ 2020-09-12 18:51                   ` Michael Heerdegen
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Heerdegen @ 2020-09-12 18:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 41145, Philipp Stephani, Mauro Aranda

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

> Hmm... so you have `define-minor-mode`s in your init file?  That
> sounds rather unusual [ And "flooded" suggests you have very many.  ]

Yes, 20.  Part of my private config, for my own usage.  I'm a bit
surprised that you think that's unusual.

> Add a dummy `defgroup` at the beginning of the file?

Ok, will give it a try.

Thanks,

Michael.





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

end of thread, other threads:[~2020-09-12 18:51 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09  8:30 bug#41145: 27.0.91; small issues with `display-fill-column-indicator' Customization group Philipp Stephani
2020-05-09  9:37 ` Eli Zaretskii
2020-05-09 12:31   ` Philipp Stephani
2020-08-28 14:48 ` Mauro Aranda
2020-08-29  6:54   ` Eli Zaretskii
2020-08-29 15:36     ` Stefan Monnier
2020-08-29 16:23       ` Mauro Aranda
2020-08-30  3:58         ` Stefan Monnier
2020-08-30 11:52           ` Mauro Aranda
2020-08-30 14:51             ` Stefan Monnier
2020-08-30 15:09               ` Eli Zaretskii
2020-08-30 17:03                 ` Stefan Monnier
2020-09-04  2:24                   ` Stefan Monnier
2020-09-04  7:09                     ` Eli Zaretskii
2020-09-04 12:47                       ` Stefan Monnier
2020-08-30 15:09               ` Drew Adams
2020-09-11 23:26               ` Michael Heerdegen
2020-09-12 15:43                 ` Stefan Monnier
2020-09-12 18:51                   ` Michael Heerdegen
2020-08-30 13:50           ` Eli Zaretskii

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