GNU bug report logs - #60102
Move gv-expander of substring to cl-lib

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60102 in the body.
You can then email your comments to 60102 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: bug-gnu-emacs <at> gnu.org
Subject: Move gv-expander of substring to cl-lib
Date: Thu, 15 Dec 2022 15:12:06 -0500

[Message part 1 (text/plain, inline)]

Tags: patch

Tags: patch

The `gv-expander` of `substring` uses the `cl--set-substring` function
that's defined only in `cl-lib`, so currently, you can compile

    (setf (substring ...) ...)

without requiring `cl-lib` but at run time it will tend to signal
a `void-function` error.
We could autoload `cl--set-substring`, but I think a better choice is to
move this `gv-expander` to `cl-lib.el`.

There are 2 other place definitions in `gv.el` which similarly rely on
helper functions defined in `cl-lib` (namely `buffer-substring` and
`frame-visible-p`) which we could move as well, but since we marked
those as obsolete anyway I think we can "let them die" where they are,
with their quirks left alone.  If you feel otherwise, I can update the
patch to move them to `cl-lib.el` as well.


        Stefan


 In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnux32, GTK+ Version
 3.24.35, cairo version 1.16.0) of 2022-12-09 built on alfajor
Repository revision: b134e7e6abf18286d38e1b589f0fdae523cf1e73
Repository branch: work
Windowing system distributor 'The X.Org Foundation', version 11.0.12101003
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure -C --enable-checking --enable-check-lisp-object-type --with-modules --with-cairo --with-tiff=ifavailable
 'CFLAGS=-Wall -g3 -Og -Wno-pointer-sign'
 PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig'

[gv-substring.patch (text/patch, attachment)]

Message #8 received at 60102 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 60102 <at> debbugs.gnu.org
Subject: Re: bug#60102: Move gv-expander of substring to cl-lib
Date: Fri, 16 Dec 2022 08:56:39 +0200

> Date: Thu, 15 Dec 2022 15:12:06 -0500
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> The `gv-expander` of `substring` uses the `cl--set-substring` function
> that's defined only in `cl-lib`, so currently, you can compile
> 
>     (setf (substring ...) ...)
> 
> without requiring `cl-lib` but at run time it will tend to signal
> a `void-function` error.
> We could autoload `cl--set-substring`, but I think a better choice is to
> move this `gv-expander` to `cl-lib.el`.
> 
> There are 2 other place definitions in `gv.el` which similarly rely on
> helper functions defined in `cl-lib` (namely `buffer-substring` and
> `frame-visible-p`) which we could move as well, but since we marked
> those as obsolete anyway I think we can "let them die" where they are,
> with their quirks left alone.  If you feel otherwise, I can update the
> patch to move them to `cl-lib.el` as well.

Is the patch for master or for the release branch?  I think it should
go to master.

Thanks.

Message #11 received at 60102 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 60102 <at> debbugs.gnu.org
Subject: Re: bug#60102: Move gv-expander of substring to cl-lib
Date: Fri, 16 Dec 2022 09:22:10 +0200

> The `gv-expander` of `substring` uses the `cl--set-substring` function
> that's defined only in `cl-lib`, so currently, you can compile
>
>     (setf (substring ...) ...)
>
> without requiring `cl-lib` but at run time it will tend to signal
> a `void-function` error.
> We could autoload `cl--set-substring`, but I think a better choice is to
> move this `gv-expander` to `cl-lib.el`.

I tried your patch by evaluating `(tab-bar-make-keymap-1)`,
but it still fails with:

Debugger entered--Lisp error: (void-function cl--set-substring)
  cl--set-substring(#("*scratch* x" 0 9 ...
  tab-bar-auto-width(((sep-1 menu-item " " ignore) (current-tab menu-item #("*scratch* x" 0 9 ...
  tab-bar-make-keymap-1()
  (progn (tab-bar-make-keymap-1))
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  command-execute(eval-last-sexp)

OTOH, autoloading `cl--set-substring` fixes this error.

Message #14 received at 60102 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60102 <at> debbugs.gnu.org
Subject: Re: bug#60102: Move gv-expander of substring to cl-lib
Date: Fri, 16 Dec 2022 09:14:01 -0500

> Is the patch for master or for the release branch?

Definitely not for the release branch, no.
So I take it you think it's OK for `master`, thanks.


        Stefan

Message #19 received at 60102 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 60102 <at> debbugs.gnu.org
Subject: Re: bug#60102: Move gv-expander of substring to cl-lib
Date: Mon, 4 Sep 2023 13:00:18 -0700

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

>> Is the patch for master or for the release branch?
>
> Definitely not for the release branch, no.
> So I take it you think it's OK for `master`, thanks.

It seems like this patch was never installed?  Should it be?

Message #22 received at 60102 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 60102 <at> debbugs.gnu.org
Subject: Re: bug#60102: Move gv-expander of substring to cl-lib
Date: Mon, 04 Sep 2023 16:43:45 -0400

>>> Is the patch for master or for the release branch?
>> Definitely not for the release branch, no.
>> So I take it you think it's OK for `master`, thanks.
> It seems like this patch was never installed?  Should it be?

Indeed, thanks, pushed,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.