* Compilation warnings in mouse.el
@ 2016-07-11 14:23 Eli Zaretskii
2016-07-11 21:55 ` Stephen Berman
0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-07-11 14:23 UTC (permalink / raw
To: Stephen Berman; +Cc: emacs-devel
Could you please fix these?
mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
fails to specify containing group
mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
fails to specify containing group
TIA
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-11 14:23 Compilation warnings in mouse.el Eli Zaretskii
@ 2016-07-11 21:55 ` Stephen Berman
2016-07-12 5:04 ` Eli Zaretskii
0 siblings, 1 reply; 23+ messages in thread
From: Stephen Berman @ 2016-07-11 21:55 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On Mon, 11 Jul 2016 17:23:47 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
> Could you please fix these?
>
> mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
> fails to specify containing group
> mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
> fails to specify containing group
Sorry about that. I saw that the other defcustoms in mouse.el have a
":group 'mouse" line and wrongly assumed this group was defined. So
should I just add ":group 'mouse" to that defcustom or instead add a
defgroup to mouse.el and remove the existing ":group 'mouse" lines?
Steve Berman
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-11 21:55 ` Stephen Berman
@ 2016-07-12 5:04 ` Eli Zaretskii
2016-07-12 8:45 ` Stephen Berman
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Eli Zaretskii @ 2016-07-12 5:04 UTC (permalink / raw
To: Stephen Berman; +Cc: emacs-devel
> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: emacs-devel@gnu.org
> Date: Mon, 11 Jul 2016 23:55:55 +0200
>
> > mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
> > fails to specify containing group
> > mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
> > fails to specify containing group
>
> Sorry about that. I saw that the other defcustoms in mouse.el have a
> ":group 'mouse" line and wrongly assumed this group was defined. So
> should I just add ":group 'mouse" to that defcustom or instead add a
> defgroup to mouse.el and remove the existing ":group 'mouse" lines?
Unlike Stefan, I think we should add :group to every defcustom.
Having them mysteriously missing from some of them is a time bomb:
remove or add enough defcustoms in the same file, and you have a bug.
Are there any downsides to having :group in all defcustoms?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 5:04 ` Eli Zaretskii
@ 2016-07-12 8:45 ` Stephen Berman
2016-07-12 22:40 ` John Wiegley
2016-07-12 22:56 ` Dmitry Gutov
2 siblings, 0 replies; 23+ messages in thread
From: Stephen Berman @ 2016-07-12 8:45 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On Tue, 12 Jul 2016 08:04:57 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: emacs-devel@gnu.org
>> Date: Mon, 11 Jul 2016 23:55:55 +0200
>>
>> > mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
>> > fails to specify containing group
>> > mouse.el:541:1:Warning: defcustom for `mouse-select-region-move-to-beginning'
>> > fails to specify containing group
>>
>> Sorry about that. I saw that the other defcustoms in mouse.el have a
>> ":group 'mouse" line and wrongly assumed this group was defined. So
>> should I just add ":group 'mouse" to that defcustom or instead add a
>> defgroup to mouse.el and remove the existing ":group 'mouse" lines?
>
> Unlike Stefan, I think we should add :group to every defcustom.
> Having them mysteriously missing from some of them is a time bomb:
> remove or add enough defcustoms in the same file, and you have a bug.
I added the :group line in commit 2f67f8a.
Steve Berman
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: Compilation warnings in mouse.el
[not found] ` <<8337nfcupy.fsf@gnu.org>
@ 2016-07-12 14:04 ` Drew Adams
0 siblings, 0 replies; 23+ messages in thread
From: Drew Adams @ 2016-07-12 14:04 UTC (permalink / raw
To: Eli Zaretskii, Stephen Berman; +Cc: emacs-devel
> Unlike Stefan, I think we should add :group to every defcustom.
> Having them mysteriously missing from some of them is a time bomb:
> remove or add enough defcustoms in the same file, and you have a bug.
100% agreement. I don't even think we should have introduced this
"shortcut" or "timesaver" feature. It serves no purpose and can
lead to problems.
> Are there any downsides to having :group in all defcustoms?
Not that I can see. I'd like to see it be required, but if it's
too late for that then I'd like to see a compiler warning at least.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 5:04 ` Eli Zaretskii
2016-07-12 8:45 ` Stephen Berman
@ 2016-07-12 22:40 ` John Wiegley
2016-07-12 22:58 ` Stefan Monnier
2016-07-12 22:56 ` Dmitry Gutov
2 siblings, 1 reply; 23+ messages in thread
From: John Wiegley @ 2016-07-12 22:40 UTC (permalink / raw
To: Eli Zaretskii; +Cc: Stephen Berman, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 454 bytes --]
>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
EZ> Unlike Stefan, I think we should add :group to every defcustom. Having
EZ> them mysteriously missing from some of them is a time bomb: remove or add
EZ> enough defcustoms in the same file, and you have a bug.
I'm with Eli on this one.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 5:04 ` Eli Zaretskii
2016-07-12 8:45 ` Stephen Berman
2016-07-12 22:40 ` John Wiegley
@ 2016-07-12 22:56 ` Dmitry Gutov
2016-07-13 14:30 ` Eli Zaretskii
2 siblings, 1 reply; 23+ messages in thread
From: Dmitry Gutov @ 2016-07-12 22:56 UTC (permalink / raw
To: Eli Zaretskii, Stephen Berman; +Cc: emacs-devel
On 07/12/2016 08:04 AM, Eli Zaretskii wrote:
> Unlike Stefan, I think we should add :group to every defcustom.
The additions to each file just have to be consistent with what's
already there. The presently discussed patch, wasn't.
Speaking of personal preference, however, I'd rather using the `mouse'
group didn't work, in the absence of a defgroup somewhere.
> Having them mysteriously missing from some of them is a time bomb:
> remove or add enough defcustoms in the same file, and you have a bug.
How would removing a defcustom, or merely adding one, lead to a bug?
The new defcustom might have :group unspecified, and if there's no
defgroup in the current file, it would become a problem, but that's
unrelated to any defcustom's already present in the file.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 22:40 ` John Wiegley
@ 2016-07-12 22:58 ` Stefan Monnier
2016-07-13 9:28 ` Joost Kremers
2016-07-13 14:31 ` Eli Zaretskii
0 siblings, 2 replies; 23+ messages in thread
From: Stefan Monnier @ 2016-07-12 22:58 UTC (permalink / raw
To: emacs-devel
EZ> Unlike Stefan, I think we should add :group to every defcustom. Having
EZ> them mysteriously missing from some of them is a time bomb:
The way I see it, defcustoms should pretty much never have :group, and
the group to which they belong is simply determined by the file in which
they occur.
E.g. the `mouse' group should be defined in mouse.el rather than in
cus-edit.el, which would have solved this problem.
EZ> remove or add
EZ> enough defcustoms in the same file, and you have a bug.
I don't see how removing/adding defcustoms in the same file
would introduce problems.
Stefan
PS: Incidentally, we have two definitions of the `mouse' group
in cus-edit.el:
% grep -A2 '(defgroup mouse ' **/*.el
lisp/cus-edit.el:(defgroup mouse nil
lisp/cus-edit.el- "Mouse support."
lisp/cus-edit.el- :group 'editing)
--
lisp/cus-edit.el:(defgroup mouse nil
lisp/cus-edit.el- "Input from the mouse."
lisp/cus-edit.el- :group 'environment)
%
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 22:58 ` Stefan Monnier
@ 2016-07-13 9:28 ` Joost Kremers
2016-07-13 12:45 ` Stefan Monnier
2016-07-13 14:31 ` Eli Zaretskii
1 sibling, 1 reply; 23+ messages in thread
From: Joost Kremers @ 2016-07-13 9:28 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
On Tue, Jul 12 2016, Stefan Monnier wrote:
> EZ> Unlike Stefan, I think we should add :group to every defcustom. Having
> EZ> them mysteriously missing from some of them is a time bomb:
>
> The way I see it, defcustoms should pretty much never have :group, and
> the group to which they belong is simply determined by the file in which
> they occur.
That would mean that in multifile packages the defcustoms must always be
put in one single file, which must be named according to the
customisation group one wants to create. And creating multiple
customisaton groups for a single package (not uncommon in larger
packages) would require multiple files.
That's to say, such a system would require that the code of a package be
organised along the lines of the customisation groups it provides. But
customisation is a UI feature, and as such does not necessarily present
the best way to organise the code.
--
Joost Kremers
Life has its moments
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 9:28 ` Joost Kremers
@ 2016-07-13 12:45 ` Stefan Monnier
2016-07-13 14:08 ` Drew Adams
0 siblings, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2016-07-13 12:45 UTC (permalink / raw
To: emacs-devel
EZ> Unlike Stefan, I think we should add :group to every defcustom. Having
EZ> them mysteriously missing from some of them is a time bomb:
>> The way I see it, defcustoms should pretty much never have :group, and
>> the group to which they belong is simply determined by the file in which
>> they occur.
> That would mean that in multifile packages the defcustoms must always be
> put in one single file,
Notice I said "pretty much". Obviously, there will be exceptions, so
we do want to keep the :group for those situations.
> which must be named according to the customisation group one wants to
> create. And creating multiple customisaton groups for a single package
> (not uncommon in larger packages) would require multiple files.
The current system (default to the last group defined in the current
file) lets you define several groups along with their defcustoms in
a single file without the use of :group.
> That's to say, such a system would require that the code of a package be
> organised along the lines of the customisation groups it provides. But
> customisation is a UI feature, and as such does not necessarily present
> the best way to organise the code.
Agreed. But in practice the most common situation is to have the
defgroup in the same file as the corresponding defcustoms.
Maybe a bit less so in multi-file packages (especially less so in
*small* multi-file packages, I think).
Stefan
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: Compilation warnings in mouse.el
2016-07-13 12:45 ` Stefan Monnier
@ 2016-07-13 14:08 ` Drew Adams
0 siblings, 0 replies; 23+ messages in thread
From: Drew Adams @ 2016-07-13 14:08 UTC (permalink / raw
To: Stefan Monnier, emacs-devel
What is the problem that _not_ requiring or warning about
a missing :group tries to solve?
Stefan: Are you encouraging this only in order to allow
laziness and slightly less verbose code, or is there an
actual problem that occurs if people consistently provide
:group with defcustoms?
"Defcustoms should pretty much never have :group" is pretty
strong ("_should_"), even if you do qualify it with "pretty
much". Why _should_ they? Is it just to be less rigid, or
is there really a _problem_ that this solves?
It's pretty strong when someone as knowledgeable and
authoritative in Emacs as yourself says that something
_should_ be some way. It helps if you say why.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 22:56 ` Dmitry Gutov
@ 2016-07-13 14:30 ` Eli Zaretskii
2016-07-13 14:41 ` Dmitry Gutov
0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-07-13 14:30 UTC (permalink / raw
To: Dmitry Gutov; +Cc: stephen.berman, emacs-devel
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 13 Jul 2016 01:56:30 +0300
>
> On 07/12/2016 08:04 AM, Eli Zaretskii wrote:
>
> > Unlike Stefan, I think we should add :group to every defcustom.
>
> The additions to each file just have to be consistent with what's
> already there. The presently discussed patch, wasn't.
But if each defcustom has its :group, then the need for consistency is
no longer a requirement, is it?
> Speaking of personal preference, however, I'd rather using the `mouse'
> group didn't work, in the absence of a defgroup somewhere.
If we can do something to that effect, I don't think I will object.
> > Having them mysteriously missing from some of them is a time bomb:
> > remove or add enough defcustoms in the same file, and you have a bug.
>
> How would removing a defcustom, or merely adding one, lead to a bug?
Well, not bug, a compilation warning that suddenly appears out of
nowhere for code that was there for a long time.
> The new defcustom might have :group unspecified, and if there's no
> defgroup in the current file, it would become a problem, but that's
> unrelated to any defcustom's already present in the file.
AFAIU, extant defcustoms could start causing warnings, given some
changes in other defcustom's. Am I mistaken?
In short, I think having one defcustom depend on another is bad for
maintenance.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-12 22:58 ` Stefan Monnier
2016-07-13 9:28 ` Joost Kremers
@ 2016-07-13 14:31 ` Eli Zaretskii
2016-07-13 15:12 ` Stefan Monnier
1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-07-13 14:31 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 12 Jul 2016 18:58:12 -0400
>
> EZ> Unlike Stefan, I think we should add :group to every defcustom. Having
> EZ> them mysteriously missing from some of them is a time bomb:
>
> The way I see it, defcustoms should pretty much never have :group, and
> the group to which they belong is simply determined by the file in which
> they occur.
But as long as such a system isn't installed, we shouldn't behave as
if it were. (And what you propose is not without downsides, I think.)
> EZ> remove or add
> EZ> enough defcustoms in the same file, and you have a bug.
>
> I don't see how removing/adding defcustoms in the same file
> would introduce problems.
We just saw such a problem, no?
> PS: Incidentally, we have two definitions of the `mouse' group
> in cus-edit.el:
Thanks. Bonus points for fixing that.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 14:30 ` Eli Zaretskii
@ 2016-07-13 14:41 ` Dmitry Gutov
0 siblings, 0 replies; 23+ messages in thread
From: Dmitry Gutov @ 2016-07-13 14:41 UTC (permalink / raw
To: Eli Zaretskii; +Cc: stephen.berman, emacs-devel
On 07/13/2016 05:30 PM, Eli Zaretskii wrote:
> But if each defcustom has its :group, then the need for consistency is
> no longer a requirement, is it?
The need for consistency is always there when one is writing code.
>> Speaking of personal preference, however, I'd rather using the `mouse'
>> group didn't work, in the absence of a defgroup somewhere.
>
> If we can do something to that effect, I don't think I will object.
We could start with a warning in Emacs 26, and make it so in 28-ish, maybe.
>> How would removing a defcustom, or merely adding one, lead to a bug?
>
> Well, not bug, a compilation warning that suddenly appears out of
> nowhere for code that was there for a long time.
For new code, AFAIU. Not for code that has been there for a while.
> AFAIU, extant defcustoms could start causing warnings, given some
> changes in other defcustom's. Am I mistaken?
I believe so.
> In short, I think having one defcustom depend on another is bad for
> maintenance.
It would be bad, if it were so. defcustom depend on defgroup.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 14:31 ` Eli Zaretskii
@ 2016-07-13 15:12 ` Stefan Monnier
2016-07-13 15:22 ` Drew Adams
2016-07-13 15:36 ` Eli Zaretskii
0 siblings, 2 replies; 23+ messages in thread
From: Stefan Monnier @ 2016-07-13 15:12 UTC (permalink / raw
To: emacs-devel
>> The way I see it, defcustoms should pretty much never have :group, and
>> the group to which they belong is simply determined by the file in which
>> they occur.
> But as long as such a system isn't installed, we shouldn't behave as
> if it were. (And what you propose is not without downsides, I think.)
Such a system has been installed ever since
commit d3b80e9b70eaa0edb4cfc0d91543c41929fa70c0
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Sun Nov 18 01:35:12 2001 +0000
(custom-current-group-alist): New var.
(custom-declare-group): Set it.
(custom-current-group): New fun.
(custom-declare-variable, custom-handle-all-keywords):
Use it as a default if no :group argument is specified.
>> I don't see how removing/adding defcustoms in the same file
>> would introduce problems.
> We just saw such a problem, no?
I must have missed something. All I saw was that someone added
a defcustom in mouse.el and did not put a :group while there is no
defgroup in that file (and all other defcustoms in there have a :group).
That seems pretty far from "removing/adding defcustoms in the same file".
>> PS: Incidentally, we have two definitions of the `mouse' group
>> in cus-edit.el:
> Thanks. Bonus points for fixing that.
Hmm... Git didn't tell me how many points I got!
BTW, I'm not really opposed to the use of :group in general.
But I'm opposed to having it be mandatory in the obvious case of
a single-file single-group package, where the :group args are just
redundant. My above 2001 commit was designed to solve that case and
it's proved to work fine since.
Stefan
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: Compilation warnings in mouse.el
2016-07-13 15:12 ` Stefan Monnier
@ 2016-07-13 15:22 ` Drew Adams
2016-07-13 15:37 ` Stefan Monnier
2016-07-13 15:36 ` Eli Zaretskii
1 sibling, 1 reply; 23+ messages in thread
From: Drew Adams @ 2016-07-13 15:22 UTC (permalink / raw
To: Stefan Monnier, emacs-devel
> BTW, I'm not really opposed to the use of :group in general.
>
> But I'm opposed to having it be mandatory in the obvious case of
> a single-file single-group package, where the :group args are just
> redundant.
Why? That's the question that has not been answered? Is it just
to allow laziness and less verbose code, or is there some real
problem that it intends to solve?
> My above 2001 commit was designed to solve that case and
> it's proved to work fine since.
"Solve?" What's the _problem_ with expecting :group when a
group is intended?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 15:12 ` Stefan Monnier
2016-07-13 15:22 ` Drew Adams
@ 2016-07-13 15:36 ` Eli Zaretskii
2016-07-13 15:42 ` Stefan Monnier
1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-07-13 15:36 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 13 Jul 2016 11:12:35 -0400
>
> >> The way I see it, defcustoms should pretty much never have :group, and
> >> the group to which they belong is simply determined by the file in which
> >> they occur.
> > But as long as such a system isn't installed, we shouldn't behave as
> > if it were. (And what you propose is not without downsides, I think.)
>
> Such a system has been installed ever since
>
> commit d3b80e9b70eaa0edb4cfc0d91543c41929fa70c0
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun Nov 18 01:35:12 2001 +0000
>
> (custom-current-group-alist): New var.
> (custom-declare-group): Set it.
> (custom-current-group): New fun.
> (custom-declare-variable, custom-handle-all-keywords):
> Use it as a default if no :group argument is specified.
No, I meant a system where one doesn't have to do anything to have a
group for a defcustom. Nothing at all.
If they must first check if there's a defgroup in the same file, and
then if there's more than one defgroup, make sure the defcustom is
after the right one, then this is an error-prone system which cannot
be trusted.
> >> I don't see how removing/adding defcustoms in the same file
> >> would introduce problems.
> > We just saw such a problem, no?
>
> I must have missed something. All I saw was that someone added
> a defcustom in mouse.el and did not put a :group while there is no
> defgroup in that file (and all other defcustoms in there have a :group).
>
> That seems pretty far from "removing/adding defcustoms in the same file".
OK, s/defcustom/defgroup/.
> But I'm opposed to having it be mandatory in the obvious case of
> a single-file single-group package, where the :group args are just
> redundant. My above 2001 commit was designed to solve that case and
> it's proved to work fine since.
IME, solving part of a problem doesn't really solve it.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 15:22 ` Drew Adams
@ 2016-07-13 15:37 ` Stefan Monnier
2016-07-13 16:07 ` Drew Adams
0 siblings, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2016-07-13 15:37 UTC (permalink / raw
To: emacs-devel
> Why? That's the question that has not been answered?
I'd expect any programmer to know that redundancy is generally a problem.
It drowns the important information, and introduces bugs when the
redundant copies aren't automatically maintained in-sync.
You call it laziness, I call it software engineering.
Stefan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 15:36 ` Eli Zaretskii
@ 2016-07-13 15:42 ` Stefan Monnier
2016-07-13 16:07 ` Drew Adams
2016-07-13 16:13 ` Eli Zaretskii
0 siblings, 2 replies; 23+ messages in thread
From: Stefan Monnier @ 2016-07-13 15:42 UTC (permalink / raw
To: emacs-devel
> No, I meant a system where one doesn't have to do anything to have a
> group for a defcustom. Nothing at all.
I guess we could do that, but I think we'd hear even more screams.
> If they must first check if there's a defgroup in the same file, and
The byte-compiler will complain if a defcustom doesn't have a :group and
there's no preceding defgroup (as was the case in the situation that
started this discussion). So I don't think it's too much of a problem.
> then if there's more than one defgroup, make sure the defcustom is
> after the right one, then this is an error-prone system which cannot
> be trusted.
Then let's restrict the current system so it also complains if there's
no :group yet there are more than one preceding defgroup.
Stefan
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: Compilation warnings in mouse.el
2016-07-13 15:42 ` Stefan Monnier
@ 2016-07-13 16:07 ` Drew Adams
2016-07-13 16:13 ` Eli Zaretskii
1 sibling, 0 replies; 23+ messages in thread
From: Drew Adams @ 2016-07-13 16:07 UTC (permalink / raw
To: Stefan Monnier, emacs-devel
> Then let's restrict the current system so it also complains if there's
> no :group yet there are more than one preceding defgroup.
Getting more complex, not less. More stuff for the user to figure
out - just what's wrong, what's the rule, what do I need to do in
this particular (multi-group) context.
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: Compilation warnings in mouse.el
2016-07-13 15:37 ` Stefan Monnier
@ 2016-07-13 16:07 ` Drew Adams
0 siblings, 0 replies; 23+ messages in thread
From: Drew Adams @ 2016-07-13 16:07 UTC (permalink / raw
To: Stefan Monnier, emacs-devel
> > Why? That's the question that has not been answered?
>
> I'd expect any programmer to know
Oh, please.
> that redundancy is generally a problem.
> It drowns the important information, and introduces bugs when the
> redundant copies aren't automatically maintained in-sync.
>
> You call it laziness, I call it software engineering.
So this aims to reduce redundancy. Thanks for making that clear.
But a :group specifies, within a given defcustom, that the option
belongs to a given group. It is not redundant. And it is "important
information."
It's a local declaration - the option's group membership is an
attribute of the option definition (defcustom), encapsulated as
part of that definition.
An alternative, if you are worried about such local declaration,
could be to declare all of the members of a group within the group
definition. (That would be worse, of course.)
What you in effect prefer is implicit zones of a file that
correspond to a given group (up to the next defgroup).
If you have only one file per group and one group per file, the
file becomes, in effect, a module for its defined options and
faces. The defgroup says that everything belongs to that group.
And if you have multiple files per group then what? Does the
first use of :group have the same effect for the files in the
group that do not have a defgroup? Or do you redundantly add
the same defgroup to each such file?
And if you have multiple groups per file then each such "module"
is a zone of the file from one defgroup to the next. (Or perhaps
to the next :group for a different group?)
This is hardly simpler, easier to read, and less error-prone, IMO.
I'd vote for the requiring at least one :group per option/face
definition.
By "require" I really mean a byte-compiler warning. I don't mind
the implicit :group you favor, as long as users also get a warning
whenever there is no :group (not just when there is none and there
is no preceding defgroup).
Let them then sort out the complexity described above, when they
get such a warning. The warning, and Emacs in general, should
encourage the use of :group - at least one per option/face, IMO.
You may call such local declaration "redundancy", if you like.
Just because you _can_ come up with a system that can save some
:group declarations (at the cost of added complexity for users
and increased error-proneness), that does not mean that's a good
idea. Such a shortcut doesn't save much and reduces clarity.
KIS.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 15:42 ` Stefan Monnier
2016-07-13 16:07 ` Drew Adams
@ 2016-07-13 16:13 ` Eli Zaretskii
2016-07-13 16:42 ` Stefan Monnier
1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2016-07-13 16:13 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 13 Jul 2016 11:42:45 -0400
>
> > If they must first check if there's a defgroup in the same file, and
>
> The byte-compiler will complain if a defcustom doesn't have a :group and
> there's no preceding defgroup (as was the case in the situation that
> started this discussion). So I don't think it's too much of a problem.
It's evidently a problem. Otherwise, I wouldn't have had to ask
Stephen to take care of those warnings.
> > then if there's more than one defgroup, make sure the defcustom is
> > after the right one, then this is an error-prone system which cannot
> > be trusted.
>
> Then let's restrict the current system so it also complains if there's
> no :group yet there are more than one preceding defgroup.
I think this cure will be worse than the decease.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Compilation warnings in mouse.el
2016-07-13 16:13 ` Eli Zaretskii
@ 2016-07-13 16:42 ` Stefan Monnier
0 siblings, 0 replies; 23+ messages in thread
From: Stefan Monnier @ 2016-07-13 16:42 UTC (permalink / raw
To: emacs-devel
>> > If they must first check if there's a defgroup in the same file, and
>> The byte-compiler will complain if a defcustom doesn't have a :group and
>> there's no preceding defgroup (as was the case in the situation that
>> started this discussion). So I don't think it's too much of a problem.
> It's evidently a problem. Otherwise, I wouldn't have had to ask
> Stephen to take care of those warnings.
As long as Git doesn't grow an AI extension which automatically guesses
that a ":group 'mouse" was meant here, I don't see how we could have
a better solution.
Stefan
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-07-13 16:42 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-11 14:23 Compilation warnings in mouse.el Eli Zaretskii
2016-07-11 21:55 ` Stephen Berman
2016-07-12 5:04 ` Eli Zaretskii
2016-07-12 8:45 ` Stephen Berman
2016-07-12 22:40 ` John Wiegley
2016-07-12 22:58 ` Stefan Monnier
2016-07-13 9:28 ` Joost Kremers
2016-07-13 12:45 ` Stefan Monnier
2016-07-13 14:08 ` Drew Adams
2016-07-13 14:31 ` Eli Zaretskii
2016-07-13 15:12 ` Stefan Monnier
2016-07-13 15:22 ` Drew Adams
2016-07-13 15:37 ` Stefan Monnier
2016-07-13 16:07 ` Drew Adams
2016-07-13 15:36 ` Eli Zaretskii
2016-07-13 15:42 ` Stefan Monnier
2016-07-13 16:07 ` Drew Adams
2016-07-13 16:13 ` Eli Zaretskii
2016-07-13 16:42 ` Stefan Monnier
2016-07-12 22:56 ` Dmitry Gutov
2016-07-13 14:30 ` Eli Zaretskii
2016-07-13 14:41 ` Dmitry Gutov
[not found] <<8360scdzik.fsf@gnu.org>
[not found] ` <<87zipnzvo4.fsf@gmx.net>
[not found] ` <<8337nfcupy.fsf@gnu.org>
2016-07-12 14:04 ` Drew Adams
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.