Omitting :group [was: master 7936c8a: * lisp/mb-depth.el (minibuffer-depth-indicator): New face.]

Omitting :group [was: master 7936c8a: * lisp/mb-depth.el (minibuffer-depth-indicator): New face.]

[Drew Adams]

> This change causes a byte-compilation warning:
>   In toplevel form:
>   mb-depth.el:38:1: Warning: defface for
>   `minibuffer-depth-indicator' fails to specify
>   containing group

IMHO, we should adopt and promote a guideline
that it's a good, not a bad, idea to always
include an explicit `:group'.

I think that Stefan is quite opposed to this,
and he even goes to the trouble of removing
"unnecessary" `:group's.  Why?

AFAIK, the only advantage to omitting `:group'
is a little less verbose code.

The advantages to _not_ omitting `:group' are:

1. Local clarity - no need to look elsewhere.

2. Ability to move the thing elsewhere - its
   behavior/meaning wrt :group won't depend on
   its location.

To me, those advantages to not omitting `:group'
far outweigh the disadvantage.  But please point
out any advantage of omitting `:group' that I
may have missed.

It's good that `:group' is optional.  But it's
also good to get in the habit of not omitting
it, IMO, and that's a good practice to encourage.

Re: Omitting :group [was: master 7936c8a: * lisp/mb-depth.el (minibuffer-depth-indicator): New face.]

[Eli Zaretskii]

> Date: Thu, 7 Jan 2021 15:41:28 +0000 (UTC)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: Juri Linkov <juri@linkov.net>, emacs-devel@gnu.org
> 
> > This change causes a byte-compilation warning:
> >   In toplevel form:
> >   mb-depth.el:38:1: Warning: defface for
> >   `minibuffer-depth-indicator' fails to specify
> >   containing group
> 
> IMHO, we should adopt and promote a guideline
> that it's a good, not a bad, idea to always
> include an explicit `:group'.

Aren't you confusing :group in defcustom with :group in defface?

RE: Omitting :group [was: master 7936c8a: * lisp/mb-depth.el (minibuffer-depth-indicator): New face.]

[Drew Adams]

> > > This change causes a byte-compilation warning:
> > >   In toplevel form:
> > >   mb-depth.el:38:1: Warning: defface for
> > >   `minibuffer-depth-indicator' fails to specify
> > >   containing group
> >
> > IMHO, we should adopt and promote a guideline
> > that it's a good, not a bad, idea to always
> > include an explicit `:group'.
> 
> Aren't you confusing :group in defcustom with :group in defface?

Perhaps I am (am I)?  What's the difference?

(elisp) `Common Keywords' suggests that its
description of :group applies to "the next few
sections-'defcustom', 'defgroup', etc."

And node `Group Definitions' suggests that a
group defined by `defgroup' applies to all of
the group's members, and that such members are
declared using `:group'.

`:group' is specified for use in many constructs,
in many nodes of the Elisp manual.  Shouldn't its
behavior (and usage guidelines) be the same
everywhere?

FWIW, I don't see any special description of 
`:group' in the doc about `defface'.  I do see,
in other nodes, cross-references to nodes that
introduce/define customization groups - but I
don't see any such xref in the node for `defface'.