unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
@ 2021-07-16  2:53 Michael Heerdegen
  2022-08-04 16:31 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2021-07-16  2:53 UTC (permalink / raw)
  To: 49592


Hello,

consider a top-level expression like this (you might want to insert this
snipped into *scratch* for testing):

(progn
  ;; comment
  ;; about that
  (define-key ...)
  )

With `which-function-mode' enabled, more or less the complete expression
is shown in the mode-line (with newlines escaped) when moving into the
expression.

The reason: `lisp-current-defun-name' doesn't check for whether the
second subexpression of a top-level expression is still on the same line
- it just returns a string including everything (i.e. all comments) in
between.

Dunno what the best fix is.  In the above case returning something like
"(progn ...)" would be best, since the second subexpression is not even
a name (a symbol).


TIA,

Michael.


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

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






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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2021-07-16  2:53 bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns Michael Heerdegen
@ 2022-08-04 16:31 ` Lars Ingebrigtsen
  2022-08-05  2:11   ` Michael Heerdegen
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-04 16:31 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> consider a top-level expression like this (you might want to insert this
> snipped into *scratch* for testing):
>
> (progn
>   ;; comment
>   ;; about that
>   (define-key ...)
>   )
>
> With `which-function-mode' enabled, more or less the complete expression
> is shown in the mode-line (with newlines escaped) when moving into the
> expression.
>
> The reason: `lisp-current-defun-name' doesn't check for whether the
> second subexpression of a top-level expression is still on the same line
> - it just returns a string including everything (i.e. all comments) in
> between.

This function is documented as:

(defun lisp-current-defun-name ()
  "Return the name of the defun at point, or nil."

There is no defun at point in this situation, so perhaps it would make
sense to return nil here?  But this is also used by add-log, so perhaps
which-func should just use something completely different and more
strict.  I.e., skip back to the top-level form, and then use the edebug
spec to pick out the name?






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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-04 16:31 ` Lars Ingebrigtsen
@ 2022-08-05  2:11   ` Michael Heerdegen
  2022-08-05  3:01     ` Michael Heerdegen
  2022-08-05 11:57     ` Lars Ingebrigtsen
  0 siblings, 2 replies; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-05  2:11 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > (progn
> >   ;; comment
> >   ;; about that
> >   (define-key ...)
> >   )
> >
> > With `which-function-mode' enabled, more or less the complete expression
> > is shown in the mode-line (with newlines escaped) when moving into the
> > expression.

> This function is documented as:
>
> (defun lisp-current-defun-name ()
>   "Return the name of the defun at point, or nil."
>
> There is no defun at point in this situation, so perhaps it would make
> sense to return nil here?

Isn't "defun" synonymous with "top-level-form" in such cases
(e.g. `end-of-defun')?

>  But this is also used by add-log, so perhaps
> which-func should just use something completely different and more
> strict.  I.e., skip back to the top-level form, and then use the edebug
> spec to pick out the name?

I don't follow - edebug-spec of what, in the above case?

Michael.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-05  2:11   ` Michael Heerdegen
@ 2022-08-05  3:01     ` Michael Heerdegen
  2022-08-05 11:57     ` Lars Ingebrigtsen
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-05  3:01 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Isn't "defun" synonymous with "top-level-form" in such cases
> (e.g. `end-of-defun')?

`add-log-current-defun-function' tells "[...] should return the
function's name as a string, or nil if point is outside a function." -
but here "function" is also meta language - for something like
"(top-level) defining form" probably.

But I don't use add-log.  In my which-function example, returning nil is
better than the current behavior.  `which-function-mode' is for
orientation, so saying "(progn ...)" would also make sense to me and
provide a bit more help than staying silent.

Michael.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-05  2:11   ` Michael Heerdegen
  2022-08-05  3:01     ` Michael Heerdegen
@ 2022-08-05 11:57     ` Lars Ingebrigtsen
  2022-08-06  0:17       ` Michael Heerdegen
  1 sibling, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-05 11:57 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Isn't "defun" synonymous with "top-level-form" in such cases
> (e.g. `end-of-defun')?

Yes, I guess.

>>  But this is also used by add-log, so perhaps
>> which-func should just use something completely different and more
>> strict.  I.e., skip back to the top-level form, and then use the edebug
>> spec to pick out the name?
>
> I don't follow - edebug-spec of what, in the above case?

I was suggesting that we could use the edebug spec (instead of
heuristics) if it exists, and otherwise fall back on something simple,
like just showing "(<first top-level symbol> ...)" if there is no edebug
spec with a `name' in it.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-05 11:57     ` Lars Ingebrigtsen
@ 2022-08-06  0:17       ` Michael Heerdegen
  2022-08-06 12:15         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-06  0:17 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > I don't follow - edebug-spec of what, in the above case?
>
> I was suggesting that we could use the edebug spec (instead of
> heuristics) if it exists,

You want to find out the correct NAME of the thing being defined by
looking up the edebug-spec of the current top-level form instead of
simply assuming that the NAME is just the first argument of the call?

Makes sense - but I don't know if it's a big win and Edebug specs help
out in enough cases (lots of macros don't have one, even in Emacs
itself, e.g. `defvar-keymap').

Anyway, it wouldn't harm and motivate people even more to write Edebug
specs.  Or do you intent to give up the heuristic of using the first
argument completely?  I don't think that would be a good idea.  Because
this heuristic works very well in practice IME.  AFAIR, most of the
time, or every time, when I saw the heuristic of
`lisp-current-defun-name' failed, the top-level expression was not a
defining form (like in my example) and `lisp-current-defun-name'
returned the complete sexp - and treating this case specially would
already give quite good results I think.

Michael.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-06  0:17       ` Michael Heerdegen
@ 2022-08-06 12:15         ` Lars Ingebrigtsen
  2022-08-07  2:09           ` Michael Heerdegen
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-06 12:15 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Makes sense - but I don't know if it's a big win and Edebug specs help
> out in enough cases (lots of macros don't have one, even in Emacs
> itself, e.g. `defvar-keymap').

I think we should consider those cases to be bugs and fix them.

> Anyway, it wouldn't harm and motivate people even more to write Edebug
> specs.

Yes, that'd be a nice side effect.

> Or do you intent to give up the heuristic of using the first
> argument completely?  I don't think that would be a good idea.

No, if there's no (usable) edebug spec, we should fall back to that
heuristic, I think.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-06 12:15         ` Lars Ingebrigtsen
@ 2022-08-07  2:09           ` Michael Heerdegen
  2022-08-08 12:32             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-07  2:09 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Hello Lars,

agreed to everything.

Michael.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-07  2:09           ` Michael Heerdegen
@ 2022-08-08 12:32             ` Lars Ingebrigtsen
  2022-08-09  2:12               ` Michael Heerdegen
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-08 12:32 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> agreed to everything.

Now done in Emacs 29.  (And feel free to adjust more if I've forgotten
something.)






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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-08 12:32             ` Lars Ingebrigtsen
@ 2022-08-09  2:12               ` Michael Heerdegen
  2022-08-09 15:29                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-09  2:12 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Now done in Emacs 29.

Thx.

> (And feel free to adjust more if I've forgotten something.)

(I just fixed the edebug-spec -> t case which error'ed.)

Should we make the "fall back: display the name of the first symbol
after opening paren" case look different from the standard case?

For which-func display in the mode-line I'd find "(progn ...)" nice,
but I guess that would not be suitable for the "add-log" case.

Michael.






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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-09  2:12               ` Michael Heerdegen
@ 2022-08-09 15:29                 ` Lars Ingebrigtsen
  2022-08-12  5:28                   ` Michael Heerdegen
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-09 15:29 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Should we make the "fall back: display the name of the first symbol
> after opening paren" case look different from the standard case?
>
> For which-func display in the mode-line I'd find "(progn ...)" nice,
> but I guess that would not be suitable for the "add-log" case.

Ah, I'd forgotten -- yes, that'd look better, but we'd have to rearrange
the calling sequence a bit since, as you say, we don't want that in the
add-log case.

Or perhaps we could add a hack like adding a text property to the string
to let which-func know that it should wrap the result in "( ...)"?






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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-09 15:29                 ` Lars Ingebrigtsen
@ 2022-08-12  5:28                   ` Michael Heerdegen
  2022-08-12 15:06                     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-12  5:28 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > For which-func display in the mode-line I'd find "(progn ...)" nice,
> > but I guess that would not be suitable for the "add-log" case.
>
> Ah, I'd forgotten -- yes, that'd look better, but we'd have to rearrange
> the calling sequence a bit since, as you say, we don't want that in the
> add-log case.
>
> Or perhaps we could add a hack like adding a text property to the string
> to let which-func know that it should wrap the result in "( ...)"?

Have you checked how other major modes handle this case?

Would also be interesting if they also all use
`add-log-current-defun-function' or if `which-func-functions' are also
used.  I would not want to special-case Elisp if not necessary.

Michael.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-12  5:28                   ` Michael Heerdegen
@ 2022-08-12 15:06                     ` Lars Ingebrigtsen
  2022-08-23 10:27                       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-12 15:06 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Have you checked how other major modes handle this case?
>
> Would also be interesting if they also all use
> `add-log-current-defun-function' or if `which-func-functions' are also
> used.  I would not want to special-case Elisp if not necessary.

The only in-tree mode that sets `which-func-functions' is python.el, so
I guess all other modes punt to `add-log-current-defun-function'.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-12 15:06                     ` Lars Ingebrigtsen
@ 2022-08-23 10:27                       ` Lars Ingebrigtsen
  2022-08-23 22:57                         ` Michael Heerdegen
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-23 10:27 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

I've now reverted these changes, because they just don't work well
enough.  As I said in the commit message:

2022-08-23  Lars Ingebrigtsen  <larsi@gnus.org>

	* lisp/emacs-lisp/lisp-mode.el (lisp-current-defun-name): Revert
	back to the old version before bug#49592.  The new approach just
	doesn't work well enough -- we don't really have the data to know
	that, say, `make-obsolete-variable' is about the second symbol and
	not the first.

So another approach is needed here.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-23 10:27                       ` Lars Ingebrigtsen
@ 2022-08-23 22:57                         ` Michael Heerdegen
  2022-08-24 10:23                           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-23 22:57 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 49592

Lars Ingebrigtsen <larsi@gnus.org> writes:

> 2022-08-23  Lars Ingebrigtsen  <larsi@gnus.org>
>
> 	* lisp/emacs-lisp/lisp-mode.el (lisp-current-defun-name): Revert
> 	back to the old version before bug#49592.  The new approach just
> 	doesn't work well enough -- we don't really have the data to know
> 	that, say, `make-obsolete-variable' is about the second symbol and
> 	not the first.

Is that because of `make-obsolete-variable's (indent defun) spec?  (And
is that even appropriate?)

Did you see examples where the edebug spec based mechanism you added
failed?  Or maybe only this part:

  (and (eq (get symbol 'lisp-indent-function) 'defun)
       (get 'defun 'edebug-form-spec))

was inappropriate?

Michael.





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

* bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns
  2022-08-23 22:57                         ` Michael Heerdegen
@ 2022-08-24 10:23                           ` Lars Ingebrigtsen
  0 siblings, 0 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-24 10:23 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49592

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Is that because of `make-obsolete-variable's (indent defun) spec?  (And
> is that even appropriate?)
>
> Did you see examples where the edebug spec based mechanism you added
> failed?  Or maybe only this part:
>
>   (and (eq (get symbol 'lisp-indent-function) 'defun)
>        (get 'defun 'edebug-form-spec))
>
> was inappropriate?

The problem wasn't just with this function -- it turns out that we have
quite a few things that are used as top-level defining constructs that
aren't marked in any special way.  The old heuristics (i.e., "just use
the second bit in the form") handle these correctly.

But that means that I have no ideas for a practical way to fix the issue
this was supposed to fix -- i.e., top-level

(progn
  ;; 
  (foo-bar)
  ...)

and such.





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

end of thread, other threads:[~2022-08-24 10:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-16  2:53 bug#49592: 28.0.50; lisp-current-defun-name and non-standard defuns Michael Heerdegen
2022-08-04 16:31 ` Lars Ingebrigtsen
2022-08-05  2:11   ` Michael Heerdegen
2022-08-05  3:01     ` Michael Heerdegen
2022-08-05 11:57     ` Lars Ingebrigtsen
2022-08-06  0:17       ` Michael Heerdegen
2022-08-06 12:15         ` Lars Ingebrigtsen
2022-08-07  2:09           ` Michael Heerdegen
2022-08-08 12:32             ` Lars Ingebrigtsen
2022-08-09  2:12               ` Michael Heerdegen
2022-08-09 15:29                 ` Lars Ingebrigtsen
2022-08-12  5:28                   ` Michael Heerdegen
2022-08-12 15:06                     ` Lars Ingebrigtsen
2022-08-23 10:27                       ` Lars Ingebrigtsen
2022-08-23 22:57                         ` Michael Heerdegen
2022-08-24 10:23                           ` Lars Ingebrigtsen

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