bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Michael Heerdegen]

Hello,

the new advertised calling contention for `define-minor-mode' is

| (mode doc &rest body)

Nothing wrong with that, but this part from the doc string:

| If you provide BODY, then you must provide at least one keyword
| argument.

says that the convention as printed is not valid - and indeed it
isn't -- e.g. this errors:

  (define-minor-mode test-mode "..." (if test-mode 17 23))

Is it possible to make that keyword-less case work?  Else the calling
convention should be made clearer in some way.

TIA, Michael.


In GNU Emacs 28.0.50 (build 48, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-04-15 built on drachen
Repository revision: ed6b86457ddf73cc2cb2df6a1cf8dab79a265a93
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

> says that the convention as printed is not valid - and indeed it
> isn't -- e.g. this errors:
>
>   (define-minor-mode test-mode "..." (if test-mode 17 23))
>
> Is it possible to make that keyword-less case work?

It's hard to do it as long as there is code out there that uses the old
convention where the first element of "body" is actually interpreted as
the `lighter` argument.

That's why I introduced the warning and changed the arglist: I realized
that even though I think the old convention is inconvenient and
confusing, it seems that a lot of code (even brand new code) follows the
new rather than the old convention, so we need to be more proactive to
get people to abandon the old convention so we can at some point in the
future accept code like the one above.


        Stefan

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

> confusing, it seems that a lot of code (even brand new code) follows the
> new rather than the old convention, so we need to be more proactive to
  ^^^                 ^^^
   \\                //
     ================ 

Sorry got them messed up.


        Stefan

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Michael Heerdegen]

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

> That's why I introduced the warning and changed the arglist: I realized
> that even though I think the old convention is inconvenient and
> confusing, it seems that a lot of code (even brand new code) follows the
> old rather than the new convention, so we need to be more proactive to
> get people to abandon the old convention so we can at some point in the
> future accept code like the one above.

Ok.


My problem was: When compiling my init file (I have a lot of personal,
quite trivial, minor modes) I got 20 warnings, a whole bulk, then I did
what the warnings said and deleted all those nil's, and then everything
was broken.

It's now hard to find out how to get your code correct and warning free
at all.  Can we do something better to help people find the correct
solution?

Regards,

Michael.

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

>> That's why I introduced the warning and changed the arglist: I realized
>> that even though I think the old convention is inconvenient and
>> confusing, it seems that a lot of code (even brand new code) follows the
>> old rather than the new convention, so we need to be more proactive to
>> get people to abandon the old convention so we can at some point in the
>> future accept code like the one above.
> My problem was: When compiling my init file (I have a lot of personal,
> quite trivial, minor modes) I got 20 warnings, a whole bulk, then I did
> what the warnings said and deleted all those nil's, and then everything
> was broken.

Hmm... the warning don't exactly say to delete those three elements, so
I think it would help to understand how you got from "saw the warning"
to "deleted all those nil's": that's the probably where we need to
improve the doc.

Another point is: after deleting those nil's, presumably you still got
the warning (because there still wasn't any keyword arg at the head), so
you should hopefully have seen that warning which would have helped
understand why "everything was broken".

> It's now hard to find out how to get your code correct and warning free
> at all.  Can we do something better to help people find the correct
> solution?

I hope we can.


        Stefan

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Michael Heerdegen]

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

> Hmm... the warning don't exactly say to delete those three elements, so
> I think it would help to understand how you got from "saw the warning"
> to "deleted all those nil's": that's the probably where we need to
> improve the doc.

What is currently the correct syntax if you can't provide any keywords
because your definition only has a body to specify?  That was the only
case that caused the problems, and I only fixed it by specifying :group
'micha (the group micha, German short form of "Michael", already exists
to quite other byte compiler warnings, and is otherwise redundant).

So that was what Emacs presented to me: a calling convention that told
to use (mode doc &rest body) which I did, and OTOH a non-decreasing
number of warnings to use keywords please , but I have none to specify,
so ... what?


Regards,

Michael.

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

>> Hmm... the warning don't exactly say to delete those three elements, so
>> I think it would help to understand how you got from "saw the warning"
>> to "deleted all those nil's": that's the probably where we need to
>> improve the doc.
>
> What is currently the correct syntax if you can't provide any keywords
> because your definition only has a body to specify?

The three args of the old convention correspond to (by order of
appearance): `:init-value`, `lighter`, `:keymap`.
So in the patch I installed into `master` I used `:lighter nil` to
replace three `nil`s (since `:init-value` and `:keymap` should only be
used in exceptional circumstances, IMO).

> So that was what Emacs presented to me: a calling convention that told
> to use (mode doc &rest body) which I did, and OTOH a non-decreasing
> number of warnings to use keywords please , but I have none to specify,
> so ... what?

Yes, I understand the problem, but I'm not sure how best to address it.
Do you have some suggestion for what could have helped you?


        Stefan

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Michael Heerdegen]

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

> Yes, I understand the problem, but I'm not sure how best to address
> it.  Do you have some suggestion for what could have helped you?

There should be some indication, somewhere, that zero keywords are
currently not allowed.

In my case, eldoc could have been such a source of information, by
letting showing something like

 (mode doc keyword1 val1 [keyword val ...] &rest body)

You could also make this warning:

"Use keywords rather than deprecated positional arguments to
`define-minor-mode'"

- or at least the docstring of `define-minor-mode' - tell that if no
keywords can be provided one can use "e.g. :lighter nil" to get rid of
the warnings (and maybe that this will probably not be necessary any
more in the future).

Apart from that, we could allow an explicit :body keyword, though that
would have been most helpful before your change, and once the sources of
examples of the old calling convention are gone, it would get completely
redundant, so I guess that is not a good proposal.


Thanks,

Michael.

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

> In my case, eldoc could have been such a source of information, by
> letting showing something like
>
>  (mode doc keyword1 val1 [keyword val ...] &rest body)

I went with

    (mode doc [keyword val ...] &rest body)

since it's allowed to have no keywords (if body is empty).

> - or at least the docstring of `define-minor-mode' - tell that if no
> keywords can be provided one can use "e.g. :lighter nil" to get rid of
> the warnings (and maybe that this will probably not be necessary any
> more in the future).

I added a mention of `:lighter nil`.


        Stefan

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Lars Ingebrigtsen]

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

> I went with
>
>     (mode doc [keyword val ...] &rest body)
>
> since it's allowed to have no keywords (if body is empty).

It still kinda looks like the keywords are optional, even if you have a
body.  What about:

  (mode doc [keyword val ... &rest body])

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

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

Lars Ingebrigtsen [2021-04-17 13:49:13] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I went with
>>
>>     (mode doc [keyword val ...] &rest body)
>>
>> since it's allowed to have no keywords (if body is empty).
>
> It still kinda looks like the keywords are optional, even if you have a
> body.  What about:
>
>   (mode doc [keyword val ... &rest body])

Done, thanks,


        Stefan

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Michael Heerdegen]

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

> Done, thanks,

I am happy with all the changes.

If nobody else has anything to contribute, you might close this report.

Thanks,

Michael.

bug#47813: 28.0.50; Confusing new calling convention for define-minor-mode

[Stefan Monnier]

Michael Heerdegen [2021-04-18 01:21:13] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Done, thanks,
> I am happy with all the changes.

Thanks, closing,


        Stefan