bug#70696: [PATCH] Autoload `re-disassemble'

bug#70696: [PATCH] Autoload `re-disassemble'

[Ihor Radchenko]

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

Tags: patch

Hi,

I have recently discovered a new `re-disassemble' command while reading
NEWS. However, the command is not available by default - one needs to
(require 'disass).  I think it should be autoloaded, just like another
function in disass.el - `disassemble'.

In GNU Emacs 30.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-05-01 built on localhost
Repository revision: cde58b309588008707cc8b00919eb24801e42eb6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Gentoo Linux

Configured using:
 'configure --enable-checking=yes,glyphs CFLAGS=-g3'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Autoload-re-disassemble.patch --]
[-- Type: text/patch, Size: 904 bytes --]

From 91ddb836b5e1594adf41f2658d4c07b053873f9d Mon Sep 17 00:00:00 2001
Message-ID: <91ddb836b5e1594adf41f2658d4c07b053873f9d.1714562831.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Wed, 1 May 2024 14:26:09 +0300
Subject: [PATCH] Autoload `re-disassemble'

* lisp/emacs-lisp/disass.el (re-disassemble): Autoload the command to
make it more discoverable.
---
 lisp/emacs-lisp/disass.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/emacs-lisp/disass.el b/lisp/emacs-lisp/disass.el
index 60881ab176b..8a4d2f6ffb9 100644
--- a/lisp/emacs-lisp/disass.el
+++ b/lisp/emacs-lisp/disass.el
@@ -292,6 +292,7 @@ disassemble-1
 	  (insert "\n")))))
   nil)
 
+;;;###autoload
 (defun re-disassemble (regexp &optional case-table)
   "Describe the compiled form of REGEXP in a separate window.
 If CASE-TABLE is non-nil, use it as translation table for case-folding.
-- 
2.44.0


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#70696: [PATCH] Autoload `re-disassemble'

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Date: Wed, 01 May 2024 11:28:48 +0000
> 
> I have recently discovered a new `re-disassemble' command while reading
> NEWS. However, the command is not available by default - one needs to
> (require 'disass).  I think it should be autoloaded, just like another
> function in disass.el - `disassemble'.

The doc string says:

  This function is mainly intended for maintenance of Emacs itself
  and may change at any time.  It requires Emacs to be built with
  `--enable-checking'."

So I'm not sure we want this auto-loaded.  Adding Mattias and Stefan
to the discussion.

bug#70696: [PATCH] Autoload `re-disassemble'

[Mattias Engdegård]

2 maj 2024 kl. 12.13 skrev Eli Zaretskii <eliz@gnu.org>:

>  This function is mainly intended for maintenance of Emacs itself
>  and may change at any time.  It requires Emacs to be built with
>  `--enable-checking'."
> 
> So I'm not sure we want this auto-loaded.

I don't think it's important to auto-load it either.

(Ihor, see it like this: the fact that you want it conveniently accessible means that you are one of a select few who have a reason to use it, and cool enough to add your own local autoload.)

bug#70696: [PATCH] Autoload `re-disassemble'

[Ihor Radchenko]

"Mattias Engdegård" <mattias.engdegard@gmail.com> writes:

>>  This function is mainly intended for maintenance of Emacs itself
>>  and may change at any time.  It requires Emacs to be built with
>>  `--enable-checking'."
>> 
>> So I'm not sure we want this auto-loaded.
>
> I don't think it's important to auto-load it either.
>
> (Ihor, see it like this: the fact that you want it conveniently accessible means that you are one of a select few who have a reason to use it, and cool enough to add your own local autoload.)

AFAIK, these kinds of helper functions are usually defined in C code and
are available without any requires as long as Emacs is compiled with
right flags. Having to do (require 'disass) was a surprise - I had to
search Emacs sources.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#70696: [PATCH] Autoload `re-disassemble'

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 70696@debbugs.gnu.org, Stefan Monnier
>  <monnier@iro.umontreal.ca>
> Date: Thu, 02 May 2024 10:27:39 +0000
> 
> "Mattias Engdegård" <mattias.engdegard@gmail.com> writes:
> 
> > (Ihor, see it like this: the fact that you want it conveniently accessible means that you are one of a select few who have a reason to use it, and cool enough to add your own local autoload.)
> 
> AFAIK, these kinds of helper functions are usually defined in C code and
> are available without any requires as long as Emacs is compiled with
> right flags. Having to do (require 'disass) was a surprise - I had to
> search Emacs sources.

I wonder why it was a surprise.  I'm quite used to the fact that some
functions and variables are not known until I load their package.  You
seem to say that it never happens to you?  How come? Emacs has gobs of
stuff that is not autoloaded, including commands and user options.

bug#70696: [PATCH] Autoload `re-disassemble'

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> AFAIK, these kinds of helper functions are usually defined in C code and
>> are available without any requires as long as Emacs is compiled with
>> right flags. Having to do (require 'disass) was a surprise - I had to
>> search Emacs sources.
>
> I wonder why it was a surprise.  I'm quite used to the fact that some
> functions and variables are not known until I load their package.  You
> seem to say that it never happens to you?  How come? Emacs has gobs of
> stuff that is not autoloaded, including commands and user options.

Not in general. I was surprised in the context of regexp-related
function. I generally have an expectation that regular expressions are
available out of the box, leading to assumption that debug functionality
for regular expressions will be available too.

That said, I am OK with either decision you make about this. It is just
_me_ who were surprised. If others are not, there is no reason to change
upstream to meet my intuition.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#70696: [PATCH] Autoload `re-disassemble'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I have recently discovered a new `re-disassemble' command while reading
> NEWS. However, the command is not available by default - one needs to
> (require 'disass).  I think it should be autoloaded, just like another
> function in disass.el - `disassemble'.

The functionality was designed under the assumption that it's used for
the benefit of those working on the regexp engine.

AFAIK you're not working on the regexp engine (are you?), so I'm curious
what you're using this function for: maybe it is more generally useful
than we thought and it deserves to be less hidden.


        Stefan

bug#70696: [PATCH] Autoload `re-disassemble'

[Ihor Radchenko]

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

> The functionality was designed under the assumption that it's used for
> the benefit of those working on the regexp engine.
>
> AFAIK you're not working on the regexp engine (are you?), so I'm curious
> what you're using this function for: maybe it is more generally useful
> than we thought and it deserves to be less hidden.

I saw the NEWS entry:

    ** New function 're-disassemble' to see the innards of a regexp.
    If you compiled with '--enable-checking', you can use this to help debug
    either your regexp performance problems or the regexp engine.

and I noticed the "regexp performance" part, which Org mode sometimes
struggles with (we just had a bug report recently where the problem lies
with way too complex regexp that makes things slow)

So, I tried to play around with this new function, attempting to see if
it can be of any use to simplify regular expressions. (Spoiler: alas, it
is not by itself useful, without knowing the internals of Emacs regexp
engine.)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#70696: [PATCH] Autoload `re-disassemble'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> So, I tried to play around with this new function, attempting to see if
> it can be of any use to simplify regular expressions. (Spoiler: alas, it
> is not by itself useful, without knowing the internals of Emacs regexp
> engine.)

Thanks.  That confirms my impression that it's useful only for people
working on the regexp engine.


        Stefan

bug#70696: [PATCH] Autoload `re-disassemble'

[Eli Zaretskii]

tags 70696 notabug wontfix
close 70696
thanks

> Cc: 70696@debbugs.gnu.org
> Date: Thu, 02 May 2024 10:40:04 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > So, I tried to play around with this new function, attempting to see if
> > it can be of any use to simplify regular expressions. (Spoiler: alas, it
> > is not by itself useful, without knowing the internals of Emacs regexp
> > engine.)
> 
> Thanks.  That confirms my impression that it's useful only for people
> working on the regexp engine.

I guess this means we don't want to autoload this command, and I'm
therefore closing this bug as wontfix.

Thanks.