bug#39539: 27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables"

bug#39539: 27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables"

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

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

Hi Eli,

The attachment is a patch for fixing the error message "Invalid function: with-connection-local-variables" when run the grep in emacs 27.
The issue can be reproduced in current version (530067463bffc982f02dcc4f2805d389704575b4) with follow steps,
1. echo -e "hello1\nhello2\nhello3" > /tmp/a
2. "M-x grep" then try  the "... -e hello /tmp/a" to grep the string

Emacs 27 will emit the error message and the grep buffer also look strange.

Please review the patch in attachment. Thank you.


[-- Attachment #2: 0001-Fix-error-message-Invalid-function-with-connection-l.patch --]
[-- Type: application/octet-stream, Size: 811 bytes --]

From c9ed3800d0b1a7554460b0a59425206d98ca8d47 Mon Sep 17 00:00:00 2001
From: "lin.sun" <lin.sun@zoom.us>
Date: Mon, 10 Feb 2020 11:45:56 +0800
Subject: [PATCH] Fix error message "Invalid function:
 with-connection-local-variables"

---
 lisp/subr.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index c1c4cad..72c11a2 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -27,6 +27,8 @@
 ;; declare-function's args use &rest, not &optional, for compatibility
 ;; with byte-compile-macroexpand-declare-function.
 
+(eval-when-compile (require 'files-x)) ;with-connection-local-variables
+
 (defmacro declare-function (_fn _file &rest _args)
   "Tell the byte-compiler that function FN is defined, in FILE.
 The FILE argument is not used by the byte-compiler, but by the
-- 
2.2.0

bug#39539: Acknowledgement (27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables")

[Sun Lin]

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

Hi Eli,

Sorry for not well formated commit message in preview patch file.
Here is the new patch file in attachment wiht the reformated commit issue. 
Please Review again, thank you.

Best Regards
Lin Sun

[-- Attachment #2: 0001-Fix-error-message-Invalid-function-with-connection-l.patch --]
[-- Type: application/octet-stream, Size: 893 bytes --]

From c4cba2617232330a1591cdaef1330e5cce7eb7b0 Mon Sep 17 00:00:00 2001
From: "lin.sun" <lin.sun@zoom.us>
Date: Mon, 10 Feb 2020 14:31:22 +0800
Subject: [PATCH] Fix error message "Invalid function:
 with-connection-local-variables"

* lisp/subr.el (start-file-process-shell-command): fix error message.
(Bug#39539)
---
 lisp/subr.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index c1c4cad..72c11a2 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -27,6 +27,8 @@
 ;; declare-function's args use &rest, not &optional, for compatibility
 ;; with byte-compile-macroexpand-declare-function.
 
+(eval-when-compile (require 'files-x)) ;with-connection-local-variables
+
 (defmacro declare-function (_fn _file &rest _args)
   "Tell the byte-compiler that function FN is defined, in FILE.
 The FILE argument is not used by the byte-compiler, but by the
-- 
2.2.0

bug#39539: Acknowledgement (27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables")

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

>Hi Eli,
>Sorry for not well formated commit message in preview patch file.
>Here is the new patch file in attachment wiht the reformated commit issue. 
>Please Review again, thank you.
Error happened with the patch when build the Emacs from source, it will be cycle require between subr-x.el and files-x.el. 
Appoligize for this incorrect patch. 
Please ignore the patch. I'll investigate more to find what happend.

bug#39539: Acknowledgement (27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables")

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

>>Hi Eli,
>>Sorry for not well formated commit message in preview patch file.
>>Here is the new patch file in attachment wiht the reformated commit issue. 
>>Please Review again, thank you.
>
>Error happened with the patch when build the Emacs from source, it will be cycle require between subr-x.el and files-x.el. 
>Appoligize for this incorrect patch. 
>Please ignore the patch. I'll investigate more to find what happend.

The issue is gone after I totally clean and rebuild Emacs27 from source.
Please close this ticket. 
Thanks.

bug#39539: Acknowledgement (27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables")

[Stefan Kangas]

Sun Lin <sunlin7@yahoo.com> writes:

> The issue is gone after I totally clean and rebuild Emacs27 from source.
> Please close this ticket.

Closed.

bug#39539: Acknowledgement (27.0.60; [PATCH] Fix error message "Invalid function: with-connection-local-variables")

[Eli Zaretskii]

> Date: Mon, 10 Feb 2020 16:04:49 +0000 (UTC)
> From: Sun Lin <sunlin7@yahoo.com>
> 
> The issue is gone after I totally clean and rebuild Emacs27 from source.
> Please close this ticket. 

Thanks, done.

bug#58127: 29.0.50; [PATCH] Fix the calc load calc-loaddefs.el without suffix sensitive

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

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

Hi,

Currently the calc.el will load the "calc-loaddefs.el" only, actually
it should be able to load the *.elc version also.
The patch just removes the .el suffix to enhance it.
Please help remove it. Thanks.

Best Regards
Lin Sun

[-- Attachment #2: 0001-calc-loaddefs-load-calc-loaddefs-without-suffix-sens.patch --]
[-- Type: text/x-patch, Size: 637 bytes --]

From 6d6463158b23ebbb485e7dc35284c267c4e871bf Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 27 Sep 2022 00:00:02 +0000
Subject: [PATCH] * calc-loaddefs: load "calc-loaddefs" without suffix
 sensitive

---
 lisp/calc/calc.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/calc/calc.el b/lisp/calc/calc.el
index c0f87ad..6ea8a42 100644
--- a/lisp/calc/calc.el
+++ b/lisp/calc/calc.el
@@ -1162,7 +1162,7 @@ calc-dispatch-map
 
 
 ;;;; (Autoloads here)
-(load "calc-loaddefs.el" nil t)
+(load "calc-loaddefs" nil t)
 
 ;;;###autoload (define-key ctl-x-map "*" 'calc-dispatch)
 
-- 
2.7.0

bug#58127: 29.0.50; [PATCH] Fix the calc load calc-loaddefs.el without suffix sensitive

[Lars Ingebrigtsen]

lin Sun <sunlin7@yahoo.com> writes:

> Currently the calc.el will load the "calc-loaddefs.el" only, actually
> it should be able to load the *.elc version also.
> The patch just removes the .el suffix to enhance it.

Thanks; pushed to Emacs 29.

bug#58129: 29.0.50; [PATCH] Fix (package-update) always install package due to invalid version comparison

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

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

The function (package-update) should do nothing if the package already
is the latest one; but it actually always installs the package.

The issue comes from the version comparison in function
(package--updateable-packages).

In function (package--updateable-packages), it will compare the
priority versions for the achieved package and the available package,
but the archived package has NO priority version, and the will get
default 0 as its priority version, then the version comparison gets
wrong results.

This patch will fix the issue, please help to review it. Thanks.

Best Regards
Lin

[-- Attachment #2: 0001-Fix-package-update-always-install-package-due-to-inv.patch --]
[-- Type: text/x-patch, Size: 1080 bytes --]

From f45d1da7e6d12ec59b23073555bce95460877845 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 27 Sep 2022 00:00:57 +0000
Subject: [PATCH] Fix (package-update) always install package due to invalid
 version comparison

* lisp/emacs-lisp/package.el (package--updateable-packages): fix
version comparison between available packages and archived packages.
---
 lisp/emacs-lisp/package.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 70c15d2..4abee9d 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2189,8 +2189,8 @@ package--updateable-packages
              (assq (car elt) package-archive-contents)))
         (and available
              (version-list-<
-              (package-desc-priority-version (cadr elt))
-              (package-desc-priority-version (cadr available))))))
+              (package-desc-version (cadr elt))
+              (package-desc-version (cadr available))))))
     package-alist)))
 
 ;;;###autoload
-- 
2.7.0

bug#58129: 29.0.50; [PATCH] Fix (package-update) always install package due to invalid version comparison

[Lars Ingebrigtsen]

lin Sun <sunlin7@yahoo.com> writes:

> This patch will fix the issue, please help to review it. Thanks.

Thanks; pushed to Emacs 29.

bug#58708: 29.0.50; [PATCH] Fix build error that gflags.will_dump_ not surround by the directives as its definition

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

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

Hi,

The "gflags.will_dump_" is defined in the lisp.h and surround by these
directives,

#if defined HAVE_PDUMPER || defined HAVE_UNEXEC

  bool will_dump_ : 1;

#endif

But its references weren't surrounded with the same directives, so
emacs will fail to build without any dumper.

This patch will fix the build errors. Please review and merge this patch. Thanks

Best Regards
Lin

[-- Attachment #2: 0001-lisp.h-suround-the-gflags.will_dump_-with-same-direc.patch --]
[-- Type: text/x-patch, Size: 1370 bytes --]

From 5b5bc154732d1f532803aae8fa2045decd38bbea Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Sat, 22 Oct 2022 00:00:06 +0000
Subject: [PATCH] lisp.h: suround the gflags.will_dump_ with same directives as
 its definition

---
 src/eval.c | 2 ++
 src/fns.c  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/eval.c b/src/eval.c
index 8810136..7e0c73b 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2300,9 +2300,11 @@ DEFUN ("autoload-do-load", Fautoload_do_load, Sautoload_do_load, 1, 3, 0,
      of what files are preloaded and when.  */
   if (will_dump_p () && !will_bootstrap_p ())
     {
+#if defined HAVE_PDUMPER || defined HAVE_UNEXEC
       /* Avoid landing here recursively while outputting the
 	 backtrace from the error.  */
       gflags.will_dump_ = false;
+#endif
       error ("Attempt to autoload %s while preparing to dump",
 	     SDATA (SYMBOL_NAME (funname)));
     }
diff --git a/src/fns.c b/src/fns.c
index 4055792..9be65de 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -3329,7 +3329,9 @@ DEFUN ("require", Frequire, Srequire, 1, 3, 0,
 	{
 	  /* Avoid landing here recursively while outputting the
 	     backtrace from the error.  */
+#if defined HAVE_PDUMPER || defined HAVE_UNEXEC
 	  gflags.will_dump_ = false;
+#endif
 	  error ("(require %s) while preparing to dump",
 		 SDATA (SYMBOL_NAME (feature)));
 	}
-- 
2.7.0

bug#58708: 29.0.50; [PATCH] Fix build error that gflags.will_dump_ not surround by the directives as its definition

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Sat, 22 Oct 2022 05:16:45 +0000
> 
> The "gflags.will_dump_" is defined in the lisp.h and surround by these
> directives,
> 
> #if defined HAVE_PDUMPER || defined HAVE_UNEXEC
> 
>   bool will_dump_ : 1;
> 
> #endif
> 
> But its references weren't surrounded with the same directives, so
> emacs will fail to build without any dumper.
> 
> This patch will fix the build errors. Please review and merge this patch. Thanks

Thanks, but I prefer to have this field to be defined unconditionally.
That's a much simpler, cleaner change, and the fact that it "wastes" a
one-bit field in some builds is nothing to worry about, IMO.

bug#58708: 29.0.50; [PATCH] Fix build error that gflags.will_dump_ not surround by the directives as its definition

[lin Sun]

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

Hi Eli,

Agree with you. And I had removed the directives that surround the
gflags.will_dump_ and the gflags.will_bootstrap_, then rebuilt emacs
with / without the dumper, it seems OK.
Please help review the patch. Thanks.

Best Regards
Lin

[-- Attachment #2: 0001-lisp.h-remove-the-directives-which-surround-the-gfla.patch --]
[-- Type: text/x-patch, Size: 795 bytes --]

From a31d8fda36d675bc51a7c4423a82a082f7716b0c Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Sat, 22 Oct 2022 00:00:06 +0000
Subject: [PATCH] lisp.h: remove the directives which surround the
 gflags.will_dump_

---
 src/lisp.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/lisp.h b/src/lisp.h
index 5f67215..1eed323 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -643,10 +643,8 @@ #define ENUM_BF(TYPE) enum TYPE
 extern struct gflags
 {
   /* True means this Emacs instance was born to dump.  */
-#if defined HAVE_PDUMPER || defined HAVE_UNEXEC
   bool will_dump_ : 1;
   bool will_bootstrap_ : 1;
-#endif
 #ifdef HAVE_PDUMPER
   /* Set in an Emacs process that will likely dump with pdumper; all
      Emacs processes may dump with pdumper, however.  */
-- 
2.7.0

bug#58708: 29.0.50; [PATCH] Fix build error that gflags.will_dump_ not surround by the directives as its definition

[Eli Zaretskii]

> From: lin Sun <sunlin7.mail@gmail.com>
> Date: Tue, 25 Oct 2022 05:32:09 +0000
> 
> Agree with you. And I had removed the directives that surround the
> gflags.will_dump_ and the gflags.will_bootstrap_, then rebuilt emacs
> with / without the dumper, it seems OK.
> Please help review the patch. Thanks.

Thanks, installed.

Btw, you use both in your email and in the patch 2 different email
accounts, none of which is recorded with your copyright assignment.
This is confusing.  Could you please use one of the email addresses
that are in your assignment?  TIA.

bug#58708: 29.0.50; [PATCH] Fix build error that gflags.will_dump_ not surround by the directives as its definition

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

Hi Eli,

>Thanks, installed.
>Btw, you use both in your email and in the patch 2 different email
>accounts, none of which is recorded with your copyright assignment.
>This is confusing.  Could you please use one of the email addresses
>that are in your assignment?  TIA.

Thank you, and I should use the mail address same as the one in the
patch. Thanks.

Best Regards
Lin

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

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

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

Hi,

The emacs will be extremely slow after I first time enable the
semantic mode and open a simple C source file which includes the
stdio.h.
I find the semantic mode will try to analyse the chained header files,
by calling the function (semantic-find-file-noselect ...) to open the
file and analyse its content, while the `find-file-hook` may have many
heavy functions.
Disabling the `find-file-hook` do have great performance improvement
for semantic mode.
Please help review this patch. Thanks

Best regards
Lin

[-- Attachment #2: 0001-semantic-fw.el-speed-up-the-semantic-find-file-nosel.patch --]
[-- Type: text/x-patch, Size: 883 bytes --]

From 7ae72df1680f405d4b8d718fb13015e002e0076d Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@yahoo.com>
Date: Sat, 29 Oct 2022 00:00:07 +0000
Subject: [PATCH] * semantic/fw.el: speed up the 'semantic-find-file-noselect'

Disabling the find-file-hook for semantic-find-file-noselect to
improve the performance on semantic chained source file analysis.
---
 lisp/cedet/semantic/fw.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/cedet/semantic/fw.el b/lisp/cedet/semantic/fw.el
index 9917c4c..cf5c60b 100644
--- a/lisp/cedet/semantic/fw.el
+++ b/lisp/cedet/semantic/fw.el
@@ -361,6 +361,8 @@ semantic-find-file-noselect
 	 (enable-local-variables :safe)
 	 ;; ... or eval variables
 	 (enable-local-eval nil)
+	 ;; also disable the find-file-hook
+	 (find-file-hook nil)
 	 )
     (save-match-data
       (find-file-noselect file nowarn rawfile wildcards))))
-- 
2.7.0

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

[Eli Zaretskii]

> Date: Sat, 29 Oct 2022 05:01:42 +0000
> From:  lin Sun via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> The emacs will be extremely slow after I first time enable the
> semantic mode and open a simple C source file which includes the
> stdio.h.
> I find the semantic mode will try to analyse the chained header files,
> by calling the function (semantic-find-file-noselect ...) to open the
> file and analyse its content, while the `find-file-hook` may have many
> heavy functions.
> Disabling the `find-file-hook` do have great performance improvement
> for semantic mode.
> Please help review this patch. Thanks
> 
> 
> From 7ae72df1680f405d4b8d718fb13015e002e0076d Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7@yahoo.com>
> Date: Sat, 29 Oct 2022 00:00:07 +0000
> Subject: [PATCH] * semantic/fw.el: speed up the 'semantic-find-file-noselect'
> 
> Disabling the find-file-hook for semantic-find-file-noselect to
> improve the performance on semantic chained source file analysis.
> ---
>  lisp/cedet/semantic/fw.el | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lisp/cedet/semantic/fw.el b/lisp/cedet/semantic/fw.el
> index 9917c4c..cf5c60b 100644
> --- a/lisp/cedet/semantic/fw.el
> +++ b/lisp/cedet/semantic/fw.el
> @@ -361,6 +361,8 @@ semantic-find-file-noselect
>  	 (enable-local-variables :safe)
>  	 ;; ... or eval variables
>  	 (enable-local-eval nil)
> +	 ;; also disable the find-file-hook
> +	 (find-file-hook nil)
>  	 )
>      (save-match-data
>        (find-file-noselect file nowarn rawfile wildcards))))
> -- 
> 2.7.0

Thanks.  But I don't see how we can override customizations of users
and modes in such a brutal way.  find-file-hook is an important method
of getting Emacs to do something when a file is visited, it is used
for many different things in Emacs.  I find the change you propose too
drastic for my palate.  We certainly risk breaking some other use
cases.

Lars, Stefan, WDYT?

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

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

> I find the semantic mode will try to analyse the chained header files,
> by calling the function (semantic-find-file-noselect ...) to open the
> file and analyse its content, while the `find-file-hook` may have many
> heavy functions.
> Disabling the `find-file-hook` do have great performance improvement
> for semantic mode.

Putting expensive code in `find-file-hook` is usually a problem
in itself.  And like Eli, I find it risky to disable it wholesale.

But the rest of code seems to call this function only if the file is not
yet visited and it always tries to kill the buffer afterwards, so really
this could just as well use a temp buffer and
`insert-file-contents` instead.

So in the end I guess the patch is OK (except for the comment, which
just paraphrases the code and thus doesn't add any information).
The other disabled thingies in that function are already similarly
problematic :-)


        Stefan

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

[Eli Zaretskii]

> Cc: 58860@debbugs.gnu.org
> Date: Sat, 29 Oct 2022 11:06:29 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > I find the semantic mode will try to analyse the chained header files,
> > by calling the function (semantic-find-file-noselect ...) to open the
> > file and analyse its content, while the `find-file-hook` may have many
> > heavy functions.
> > Disabling the `find-file-hook` do have great performance improvement
> > for semantic mode.
> 
> Putting expensive code in `find-file-hook` is usually a problem
> in itself.  And like Eli, I find it risky to disable it wholesale.
> 
> But the rest of code seems to call this function only if the file is not
> yet visited and it always tries to kill the buffer afterwards, so really
> this could just as well use a temp buffer and
> `insert-file-contents` instead.
> 
> So in the end I guess the patch is OK (except for the comment, which
> just paraphrases the code and thus doesn't add any information).
> The other disabled thingies in that function are already similarly
> problematic :-)

It all made sense to me, until the last paragraph: it seems to
contradict the other two?  You say that disabling find-file-hook is
dangerous, and that the code in question could have used
insert-file-contents into a temporary buffer, in which case
find-file-hook would not come into play.  So I assumed you'd suggest
to rewrite that function to use a temporary buffer.  Instead, you say
that the patch is fine?..  What did I miss?

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

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

> It all made sense to me, until the last paragraph: it seems to
> contradict the other two?  You say that disabling find-file-hook is
> dangerous, and that the code in question could have used
> insert-file-contents into a temporary buffer, in which case
> find-file-hook would not come into play.  So I assumed you'd suggest
> to rewrite that function to use a temporary buffer.  Instead, you say
> that the patch is fine?..  What did I miss?

Yes, a better patch is probably to use `insert-file-contents`, but that
might involve further non trivial changes (e.g. I don't know if the
rest of the CEDET code relies on the fact that `find-buffer-visiting`
will find those buffers).  The patch is a minor tweak to the code which
brings it yet a bit closer to a version of the code that uses just
`insert-file-contents`, so it's not ideal but it doesn't make the
code any worse.


        Stefan

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

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

Hi Stefan, Eli,

Thanks for your comments, and agree with you.
I'll check my local `find-file-hook` first, and remove the heavy functions.
For using with-temp-buffer and insert-file-contents together, maybe
just a little improvement, not the key point.
So, please close this ticket, and if I find other significant
enhancements, I'll send a new patch. Thank you.

Best Regard
Lin

bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Sun, 30 Oct 2022 17:13:18 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 58860@debbugs.gnu.org
> 
> Hi Stefan, Eli,
> 
> Thanks for your comments, and agree with you.
> I'll check my local `find-file-hook` first, and remove the heavy functions.
> For using with-temp-buffer and insert-file-contents together, maybe
> just a little improvement, not the key point.
> So, please close this ticket, and if I find other significant
> enhancements, I'll send a new patch. Thank you.

Thanks, closing.

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

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

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

Hi,

The patch 8638aace3f try to use the variable
`byte-compile-ignore-files`, but the logic is inverse for both file
and directory parts;
and the patch a01024c859 fix the logic for file part;
this patch try to fix the logic for directory part.

Please help review and apply it. Thanks.

Best regards
Lin

[-- Attachment #2: 0001-bytecomp.el-byte-recompile-directory-Fix-negated-ign.patch --]
[-- Type: text/x-patch, Size: 1497 bytes --]

From 1e00f1ab7ea8171e99068486d6b93ba62e905777 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@yahoo.com>
Date: Sun, 13 Nov 2022 00:00:22 +0000
Subject: [PATCH] * bytecomp.el: (byte-recompile-directory): Fix negated ignore
 logic

Should not append the sub-directories when match the
byte-compile-ignore-files regular expression.  (Bug#59115)
---
 lisp/emacs-lisp/bytecomp.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index c685e5087f..f11c4efc6c 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1942,11 +1942,11 @@ byte-recompile-directory
 		      (or (null arg) (eq 0 arg)
 			  (y-or-n-p (concat "Check " source "? ")))
 		      (setq directories (nconc directories (list source)))
-                      ;; Directory is requested to be ignored
-                      (string-match-p
-                       (regexp-opt byte-compile-ignore-files)
-                       source)
-                      (setq directories (nconc directories (list source))))
+		      ;; Directory is requested to be ignored
+		      (not (string-match-p
+			    (regexp-opt byte-compile-ignore-files)
+			    source))
+		      (setq directories (nconc directories (list source))))
                ;; It is an ordinary file.  Decide whether to compile it.
                (if (and (string-match emacs-lisp-file-regexp source)
 			;; The next 2 tests avoid compiling lock files
-- 
2.20.5

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

[Eli Zaretskii]

merge 59236 59139
thanks

> Date: Sun, 13 Nov 2022 04:26:45 +0000
> From:  lin Sun via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> The patch 8638aace3f try to use the variable
> `byte-compile-ignore-files`, but the logic is inverse for both file
> and directory parts;
> and the patch a01024c859 fix the logic for file part;
> this patch try to fix the logic for directory part.
> 
> Please help review and apply it. Thanks.

This is a duplicate of bug#59139.

Philip, can you please take care of this?

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

> merge 59236 59139
> thanks
>
>> Date: Sun, 13 Nov 2022 04:26:45 +0000
>> From:  lin Sun via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> The patch 8638aace3f try to use the variable
>> `byte-compile-ignore-files`, but the logic is inverse for both file
>> and directory parts;
>> and the patch a01024c859 fix the logic for file part;
>> this patch try to fix the logic for directory part.
>> 
>> Please help review and apply it. Thanks.
>
> This is a duplicate of bug#59139.
>
> Philip, can you please take care of this?

I had applied the patch that was identical to the one proposed here, but
it appears my message confirming that wasn't sent out.  I'll update both
bug reports.

bug#59139: 29.0.50; batch-byte-recompile-directory doesn't recompile file as expected

[David Ponce]

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

Hello,

I recently observed that batch-byte-recompile-directory no more
recompile .el files newer than their .elc file.

Here is the shell command I use:

emacs -batch --no-init-file --no-site-file --eval "(setq load-path [my
load path])" -f batch-byte-recompile-directory .

After some investigation, I found that commit
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8638aace3fbe01529f33870f469fa60bf5e43ee7
could be the culprit.

The attached patch fixed the above commit and solved the issue for me.

Thanks

In GNU Emacs 29.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version
  3.24.34, cairo version 1.17.6) of 2022-11-07 built on kilauea
Repository revision: d04433b96215d7d3387573f19cc315de86f2341a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 36 (KDE Plasma)

Configured using:
  'configure --prefix=/home/dponce --with-x-toolkit=gtk3
  PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'

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

Important settings:
   value of $LC_TIME: fr_FR.utf8
   value of $LANG: fr_FR.UTF-8
   locale-coding-system: utf-8-unix

[-- Attachment #2: bytecomp.el.patch --]
[-- Type: text/x-patch, Size: 1623 bytes --]

index 4d258da..c81c42e 100644
--- a/emacs/lisp/emacs-lisp/bytecomp.el
+++ b/bytecomp.el
@@ -1941,11 +1941,10 @@ also be compiled."
 		      ;; This file is a subdirectory.  Handle them differently.
 		      (or (null arg) (eq 0 arg)
 			  (y-or-n-p (concat "Check " source "? ")))
-		      (setq directories (nconc directories (list source)))
                       ;; Directory is requested to be ignored
-                      (string-match-p
-                       (regexp-opt byte-compile-ignore-files)
-                       source)
+                      (not (string-match-p
+                            (regexp-opt byte-compile-ignore-files)
+                            source))
                       (setq directories (nconc directories (list source))))
                ;; It is an ordinary file.  Decide whether to compile it.
                (if (and (string-match emacs-lisp-file-regexp source)
@@ -1955,9 +1954,9 @@ also be compiled."
                         (not (auto-save-file-name-p source))
                         (not (member source (dir-locals--all-files directory)))
                         ;; File is requested to be ignored
-                        (string-match-p
-                         (regexp-opt byte-compile-ignore-files)
-                         source))
+                        (not (string-match-p
+                              (regexp-opt byte-compile-ignore-files)
+                              source)))
                    (progn (cl-incf
                            (pcase (byte-recompile-file source force arg)
                              ('no-byte-compile skip-count)

bug#59139: Acknowledgement (29.0.50; batch-byte-recompile-directory doesn't recompile file as expected)

[David Ponce]

Part of the issue (Bug#59115) has just been fixed by commit 
a01024c859fd98a4a330a9b627dc11232afc6ad0

However it seems my proposed patch is more complete.

Thanks!

bug#59139: 29.0.50; batch-byte-recompile-directory doesn't recompile file as expected

[David Ponce]

Hello,

If I correctly understood the code, a fix similar to the one applied for Bug#59115
is also needed for directories.  Please find below another patch.
Please also feel free to close this Bug#59139.

diff --git a/bytecomp.el b/bytecomp.el
index c685e50..c81c42e 100644
--- a/bytecomp.el
+++ b/bytecomp.el
@@ -1941,11 +1941,10 @@ also be compiled."
  		      ;; This file is a subdirectory.  Handle them differently.
  		      (or (null arg) (eq 0 arg)
  			  (y-or-n-p (concat "Check " source "? ")))
-		      (setq directories (nconc directories (list source)))
                        ;; Directory is requested to be ignored
-                      (string-match-p
-                       (regexp-opt byte-compile-ignore-files)
-                       source)
+                      (not (string-match-p
+                            (regexp-opt byte-compile-ignore-files)
+                            source))
                        (setq directories (nconc directories (list source))))
                 ;; It is an ordinary file.  Decide whether to compile it.
                 (if (and (string-match emacs-lisp-file-regexp source)


Thanks!

bug#59139: closed (Re: bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore)

[David Ponce]

On 13/11/2022 08:23, GNU bug Tracking System wrote:
> Your bug report
> 
> #59236: 29.0.50; batch-byte-recompile-directory doesn't recompile file as expected
> 
> which was filed against the emacs package, has been closed.
> 
> The explanation is attached below, along with your original report.
> If you require more details, please reply to 59139@debbugs.gnu.org.
> 

As mentioned in bug#59236 and my reported bug#59139, the patch
8638aace3f to use the variable `byte-compile-ignore-files` has
the logic inverse for both file and *directory* parts.
Currently the patch a01024c859 fixed the logic for file part, but
the fix to the directory part is still missing, as far as I can
see in master.  Currently, according to the code, when a
directory matchs `byte-compile-ignore-files` it is appended 2
times instead of being ignored.

Thanks

bug#59139: closed (Re: bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore)

[Philip Kaludercic]

David Ponce <da_vid@orange.fr> writes:

> On 13/11/2022 08:23, GNU bug Tracking System wrote:
>> Your bug report
>> #59236: 29.0.50; batch-byte-recompile-directory doesn't recompile
>> file as expected
>> which was filed against the emacs package, has been closed.
>> The explanation is attached below, along with your original report.
>> If you require more details, please reply to 59139@debbugs.gnu.org.
>> 
>
> As mentioned in bug#59236 and my reported bug#59139, the patch
> 8638aace3f to use the variable `byte-compile-ignore-files` has
> the logic inverse for both file and *directory* parts.
> Currently the patch a01024c859 fixed the logic for file part, but
> the fix to the directory part is still missing, as far as I can
> see in master.  Currently, according to the code, when a
> directory matchs `byte-compile-ignore-files` it is appended 2
> times instead of being ignored.

You are right.  I have also discovered that this is related to the
failing tests reported in bug#59109.  I'll rework your patch and apply
that then.

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

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

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

Look forward for the fix. Thanks!

[-- Attachment #2: Type: text/html, Size: 59 bytes --]

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

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

Hi,

Didn't see a path on the master  branch.
Can any one help apply the patch to fix byte-recompile-directory. Thanks

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

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

Hi,

Just ping anyone available to apply the patch for this issue. Thanks

> Didn't see a path on the master  branch.
> Can any one help apply the patch to fix byte-recompile-directory. Thanks

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

[Philip Kaludercic]

lin Sun <sunlin7@yahoo.com> writes:

> Hi,
>
> Just ping anyone available to apply the patch for this issue. Thanks
>
>> Didn't see a path on the master  branch.
>> Can any one help apply the patch to fix byte-recompile-directory. Thanks

I have applied the patch complete on a branch I am currently working on
that is related to this issue and bug#59109.  The branch should be
merged any day now.

bug#59236: 29.0.50; [PATCH] bytecomp.el: (byte-recompile-directory): Fix negated ignore

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

Great, thanks for the update.
Look forward for the patch, I have local script failed on it : )

bug#60209: 29.0.50; [PATCH] lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el

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

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

Hi,

I got a build error with last commit 2b1fdbffcb595bcd72fa9aa3db674c6985042bcb,

error ("Version must be a string")
  error("Version must be a string")
  version-to-list(29.1)
  version<(29.1 "19.29")
  #f(compiled-function (e1 e2) #<bytecode 0x7353c27725409c8>)((29.1
ruby-mode) ("19.29" time-stamp))

This patch will fix the error. Please merge it, thanks.

Best regards
Lin

[-- Attachment #2: 0001-ruby-mode.el-fix-the-version-should-be-a-string.patch --]
[-- Type: text/x-patch, Size: 854 bytes --]

From e22ffc678d7072550274f6dc21d839e3579953a9 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@yahoo.com>
Date: Mon, 19 Dec 2022 22:31:36 +0000
Subject: [PATCH] * ruby-mode.el: fix the version should be a string

---
 lisp/progmodes/ruby-mode.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 2b813dfcbcc..1f3e9b6ae7b 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -283,7 +283,7 @@ ruby-method-params-indent
                  (number :tag "Indent specified number of columns against def")
                  (const :tag "Indent to def" nil))
   :safe (lambda (val) (or (memq val '(t nil)) (numberp val)))
-  :version 29.1)
+  :version "29.1")
 
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
-- 
2.20.5

bug#62609: 29.0.60; [PATCH] src/comp.c: New variable `comp-el-to-eln-strip-prefix` for `comp-el-to-eln-rel-filename`

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

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

Hi,

The emacs with native compile enabled will always rebuild all the
~/.emacs.d/eln-cache/VER/*.eln files if I moved/copied eln-cache
folder from
/home/userA/.emacs.d/eln-cache/ to
/home/userB/.emacs.d/eln-cache/.

It is caused by the function `comp-el-to-eln-rel-filename` calculating
the hash value from the absolute *.el filename to be the middle of the
eln file name.

The new variable `comp-el-to-eln-strip-prefix` in this patch will
allow moving the eln-cache/ folder without rebuilding *.eln in it.

Please help review this change, Thanks

Best Regards
Lin

[-- Attachment #2: 0001-New-variable-comp-el-to-eln-strip-prefix-for-comp-el.patch --]
[-- Type: text/x-patch, Size: 3181 bytes --]

From ae6a941236044d64e2e8f88f66739149f5260394 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Sun, 2 Apr 2023 00:00:09 +0000
Subject: [PATCH] New variable `comp-el-to-eln-strip-prefix` for
 `comp-el-to-eln-rel-filename`

* src/comp.c: define the variable `comp-el-to-eln-strip-prefix`
* emacs-lisp/comp.el: forward the variable to native compile workers
---
 lisp/emacs-lisp/comp.el |  1 +
 src/comp.c              | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index e97832455b9..6c9ccb247ca 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -3996,6 +3996,7 @@ comp-run-async-workers
                                              native-comp-driver-options
                                              load-path
                                              backtrace-line-length
+                                             comp-el-to-eln-strip-prefix
                                              ;; package-load-list
                                              ;; package-user-dir
                                              ;; package-directory-list
diff --git a/src/comp.c b/src/comp.c
index 3f72d088a66..ead1171b001 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -4400,7 +4400,11 @@ DEFUN ("comp-el-to-eln-rel-filename", Fcomp_el_to_eln_rel_filename,
 and Emacs must have been compiled with zlib; the file will be
 uncompressed on the fly to hash its contents.
 Value includes the original base name, followed by 2 hash values,
-one for the file name and another for its contents, followed by .eln.  */)
+one for the file name and another for its contents, followed by .eln.
+
+The file part hash value is generated from the absolute file path, however,
+the absolute path can be stripped with `comp-el-to-eln-strip-prefix' to left
+the significant part for hashing.  */)
   (Lisp_Object filename)
 {
   CHECK_STRING (filename);
@@ -4462,7 +4466,7 @@ DEFUN ("comp-el-to-eln-rel-filename", Fcomp_el_to_eln_rel_filename,
       loadsearch_re_list = list2 (sys_re, Fregexp_quote (dump_load_search));
     }
 
-  Lisp_Object lds_re_tail = loadsearch_re_list;
+  Lisp_Object lds_re_tail = CALLN (Fappend, Vcomp_el_to_eln_strip_prefix, loadsearch_re_list);
   FOR_EACH_TAIL (lds_re_tail)
     {
       Lisp_Object match_idx =
@@ -5864,6 +5868,15 @@ syms_of_comp (void)
      dump reload.  */
   Vnative_comp_eln_load_path = Fcons (build_string ("../native-lisp/"), Qnil);
 
+  DEFVAR_LISP ("comp-el-to-eln-strip-prefix", Vcomp_el_to_eln_strip_prefix,
+    doc: /* List of regex to strip the path prefix in the function
+`comp-el-to-eln-rel-filename'.
+
+When the `comp-el-to-eln-rel-filename' try to convert an <absolute-path>.el
+to eln file name, it will remove the matched prefix on this list and hash
+the rest part to be middle of eln file name, this will allow to move the
+eln-cache/ directory without rebuilding the *.eln files in it.  */);
+
   DEFVAR_LISP ("native-comp-enable-subr-trampolines",
 	       Vnative_comp_enable_subr_trampolines,
     doc: /* If non-nil, enable generation of trampolines for calling primitives.
-- 
2.20.5

bug#62609: 29.0.60; [PATCH] src/comp.c: New variable `comp-el-to-eln-strip-prefix` for `comp-el-to-eln-rel-filename`

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Sun, 2 Apr 2023 05:13:28 +0000
> 
> The emacs with native compile enabled will always rebuild all the
> ~/.emacs.d/eln-cache/VER/*.eln files if I moved/copied eln-cache
> folder from
> /home/userA/.emacs.d/eln-cache/ to
> /home/userB/.emacs.d/eln-cache/.
> 
> It is caused by the function `comp-el-to-eln-rel-filename` calculating
> the hash value from the absolute *.el filename to be the middle of the
> eln file name.

Thanks.

However, I'm not sure I understand: the names of the *.eln files
reflect the absolute name of the *.el file, so the *.eln files will be
regenerated when the *.el files are moved, not when the eln-cache
directory is moved.  Right?  Then what is the problem you are trying
to fix?

> The new variable `comp-el-to-eln-strip-prefix` in this patch will
> allow moving the eln-cache/ folder without rebuilding *.eln in it.

Sorry, I don't think I agree to this change (assuming I understand it
correctly).  We encode the absolute name of the *.el files in the
names of the *.eln files for a reason, so ignoring arbitrary parts of
those absolute names will lead to dangerous clashes.  Please keep in
mind that, unlike with *.elc files, loading an incorrect *.eln files
can crash Emacs.  So we must be very cautious in which files we allow
to load.

And again, I don't think I understand why moving eln-cache without
moving the source *.el files would require the *.eln files in
eln-cache to be recompiled.  Andrea, what am I missing here?

bug#62609: 29.0.60; [PATCH] src/comp.c: New variable `comp-el-to-eln-strip-prefix` for `comp-el-to-eln-rel-filename`

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

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

Hi Eli,

Apologize for my wrong comment.
You're right, the patch will allow to relocate/copy an elisp source folder from
/home/userA/.emacs.d/epla/**.el to
/home/userB/.emacs.d/epla/**.el.
So the patch should be the
"0002-New-variable-comp-el-to-eln-strip-prefix-for-comp-el.patch"
attached in this email.

And agree with you, it's dangerous when this variable is configured
with an arbitrary value.
So please feel free to close this ticket. Thanks

Best regards
Lin

[-- Attachment #2: 0002-New-variable-comp-el-to-eln-strip-prefix-for-comp-el.patch --]
[-- Type: text/x-patch, Size: 3181 bytes --]

From 78cfbda547678a1c655c8c3c06059b6d16696380 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Sun, 2 Apr 2023 00:00:09 +0000
Subject: [PATCH] New variable `comp-el-to-eln-strip-prefix` for
 `comp-el-to-eln-rel-filename`

* src/comp.c: define the variable `comp-el-to-eln-strip-prefix`
* emacs-lisp/comp.el: forward the variable to native compile workers
---
 lisp/emacs-lisp/comp.el |  1 +
 src/comp.c              | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index e97832455b9..6c9ccb247ca 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -3996,6 +3996,7 @@ comp-run-async-workers
                                              native-comp-driver-options
                                              load-path
                                              backtrace-line-length
+                                             comp-el-to-eln-strip-prefix
                                              ;; package-load-list
                                              ;; package-user-dir
                                              ;; package-directory-list
diff --git a/src/comp.c b/src/comp.c
index 3f72d088a66..d464616ff3e 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -4400,7 +4400,11 @@ DEFUN ("comp-el-to-eln-rel-filename", Fcomp_el_to_eln_rel_filename,
 and Emacs must have been compiled with zlib; the file will be
 uncompressed on the fly to hash its contents.
 Value includes the original base name, followed by 2 hash values,
-one for the file name and another for its contents, followed by .eln.  */)
+one for the file name and another for its contents, followed by .eln.
+
+The file part hash value is generated from the absolute file path, however,
+the absolute path can be stripped with `comp-el-to-eln-strip-prefix' to left
+the significant part for hashing.  */)
   (Lisp_Object filename)
 {
   CHECK_STRING (filename);
@@ -4462,7 +4466,7 @@ DEFUN ("comp-el-to-eln-rel-filename", Fcomp_el_to_eln_rel_filename,
       loadsearch_re_list = list2 (sys_re, Fregexp_quote (dump_load_search));
     }
 
-  Lisp_Object lds_re_tail = loadsearch_re_list;
+  Lisp_Object lds_re_tail = CALLN (Fappend, Vcomp_el_to_eln_strip_prefix, loadsearch_re_list);
   FOR_EACH_TAIL (lds_re_tail)
     {
       Lisp_Object match_idx =
@@ -5864,6 +5868,15 @@ syms_of_comp (void)
      dump reload.  */
   Vnative_comp_eln_load_path = Fcons (build_string ("../native-lisp/"), Qnil);
 
+  DEFVAR_LISP ("comp-el-to-eln-strip-prefix", Vcomp_el_to_eln_strip_prefix,
+    doc: /* List of regex to strip the path prefix in the function
+`comp-el-to-eln-rel-filename'.
+
+When the `comp-el-to-eln-rel-filename' try to convert an <absolute-path>.el
+to eln file name, it will remove the matched prefix on this list and hash
+the rest part to be middle of eln file name, this will allow to relocate the
+*.el source file without rebuild the relative *.eln files.  */);
+
   DEFVAR_LISP ("native-comp-enable-subr-trampolines",
 	       Vnative_comp_enable_subr_trampolines,
     doc: /* If non-nil, enable generation of trampolines for calling primitives.
-- 
2.20.5

bug#62609: 29.0.60; [PATCH] src/comp.c: New variable `comp-el-to-eln-strip-prefix` for `comp-el-to-eln-rel-filename`

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Mon, 3 Apr 2023 00:53:18 +0000
> 
> You're right, the patch will allow to relocate/copy an elisp source folder from
> /home/userA/.emacs.d/epla/**.el to
> /home/userB/.emacs.d/epla/**.el.
> So the patch should be the
> "0002-New-variable-comp-el-to-eln-strip-prefix-for-comp-el.patch"
> attached in this email.
> 
> And agree with you, it's dangerous when this variable is configured
> with an arbitrary value.
> So please feel free to close this ticket. Thanks

I'd like to hear what Andrea thinks about this, before I decide
whether to close.

bug#62609: 29.0.60; [PATCH] src/comp.c: New variable `comp-el-to-eln-strip-prefix` for `comp-el-to-eln-rel-filename`

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: lin Sun <sunlin7@yahoo.com>
>> Date: Mon, 3 Apr 2023 00:53:18 +0000
>> 
>> You're right, the patch will allow to relocate/copy an elisp source folder from
>> /home/userA/.emacs.d/epla/**.el to
>> /home/userB/.emacs.d/epla/**.el.
>> So the patch should be the
>> "0002-New-variable-comp-el-to-eln-strip-prefix-for-comp-el.patch"
>> attached in this email.
>> 
>> And agree with you, it's dangerous when this variable is configured
>> with an arbitrary value.
>> So please feel free to close this ticket. Thanks
>
> I'd like to hear what Andrea thinks about this, before I decide
> whether to close.

Hi all,

I agree we should not add a new knob in order to move eln-cache without
moving the source *.el.  The use-case sounds like a corner case and this
new variable would be IMO very sensitive to miss configurations.

Best Regards

  Andrea

bug#62609: 29.0.60; [PATCH] src/comp.c: New variable `comp-el-to-eln-strip-prefix` for `comp-el-to-eln-rel-filename`

[Eli Zaretskii]

tags 62609 wontfix
thanks

> From: Andrea Corallo <akrl@sdf.org>
> Cc: lin Sun <sunlin7@yahoo.com>, 62609@debbugs.gnu.org
> Date: Mon, 03 Apr 2023 14:08:36 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: lin Sun <sunlin7@yahoo.com>
> >> Date: Mon, 3 Apr 2023 00:53:18 +0000
> >> 
> >> You're right, the patch will allow to relocate/copy an elisp source folder from
> >> /home/userA/.emacs.d/epla/**.el to
> >> /home/userB/.emacs.d/epla/**.el.
> >> So the patch should be the
> >> "0002-New-variable-comp-el-to-eln-strip-prefix-for-comp-el.patch"
> >> attached in this email.
> >> 
> >> And agree with you, it's dangerous when this variable is configured
> >> with an arbitrary value.
> >> So please feel free to close this ticket. Thanks
> >
> > I'd like to hear what Andrea thinks about this, before I decide
> > whether to close.
> 
> Hi all,
> 
> I agree we should not add a new knob in order to move eln-cache without
> moving the source *.el.  The use-case sounds like a corner case and this
> new variable would be IMO very sensitive to miss configurations.

OK, so closing.

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

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

Hi,

The package.el will always require the entire `info.el` on its package
activating function.
Using the `with-eval-after-load` to arrange the work can avoid the
unnecessary loading the `info` package.

Please help review the patch. Thanks.

Best regards
Lin

[-- Attachment #2: 0001-lisp-emacs-lisp-package.el-set-variables-after-info-.patch --]
[-- Type: text/x-patch, Size: 1066 bytes --]

From 4908999eefa315cfa1a1434baf891ce98ee8d871 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] *lisp/emacs-lisp/package.el: set variables after info package
 be loaded

---
 lisp/emacs-lisp/package.el | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index f92afe56b7..bb0491e509 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -892,10 +892,9 @@ package-activate-1
         (add-to-list 'load-path (directory-file-name pkg-dir)))
       ;; Add info node.
       (when (file-exists-p (expand-file-name "dir" pkg-dir))
-        ;; FIXME: not the friendliest, but simple.
-        (require 'info)
-        (info-initialize)
-        (add-to-list 'Info-directory-list pkg-dir))
+        (with-eval-after-load 'info
+          (info-initialize)
+          (add-to-list 'Info-directory-list pkg-dir)))
       (push name package-activated-list)
       ;; Don't return nil.
       t)))
-- 
2.20.5

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Philip Kaludercic]

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

lin Sun <sunlin7@yahoo.com> writes:

> Hi,
>
> The package.el will always require the entire `info.el` on its package
> activating function.
> Using the `with-eval-after-load` to arrange the work can avoid the
> unnecessary loading the `info` package.

Note that this is not the only place where info is required.  If this is
an acceptable solution (which I am not sure since `with-eval-after-load'
is something I usually encounter in user configurations), then we should
think about consistently addressing the issue:


[-- Attachment #2: Type: text/plain, Size: 1405 bytes --]

3 matches for "require 'info" in buffer: package.el
       :        (add-to-list 'load-path (directory-file-name pkg-dir)))
       :      ;; Add info node.
       :      (when (file-exists-p (expand-file-name "dir" pkg-dir))
       :        ;; FIXME: not the friendliest, but simple.
    896:        (require 'info)
       :        (info-initialize)
       :        (add-to-list 'Info-directory-list pkg-dir))
       :      (push name package-activated-list)
       :      ;; Don't return nil.
-------
       :(defun package-quickstart-refresh ()
       :  "(Re)Generate the `package-quickstart-file'."
       :  (interactive)
       :  (package-initialize 'no-activate)
   4444:  (require 'info)
       :  (let ((package--quickstart-pkgs ())
       :        ;; Pretend we haven't activated anything yet!
       :        (package-activated-list ())
       :        ;; Make sure we can load this file without load-source-file-function.
-------
       :                          package-activated-list)))
       :          (current-buffer))
       :      (let ((info-dirs (butlast Info-directory-list)))
       :        (when info-dirs
   4489:          (pp `(progn (require 'info)
       :                      (info-initialize)
       :                      (setq Info-directory-list
       :                            (append ',info-dirs Info-directory-list)))
       :              (current-buffer))))

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



> Please help review the patch. Thanks.
>
> Best regards
> Lin
>
> From 4908999eefa315cfa1a1434baf891ce98ee8d871 Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7@hotmail.com>
> Date: Tue, 11 Apr 2023 00:00:13 +0000
> Subject: [PATCH] *lisp/emacs-lisp/package.el: set variables after info package
>  be loaded
>
> ---
>  lisp/emacs-lisp/package.el | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index f92afe56b7..bb0491e509 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -892,10 +892,9 @@ package-activate-1
>          (add-to-list 'load-path (directory-file-name pkg-dir)))
>        ;; Add info node.
>        (when (file-exists-p (expand-file-name "dir" pkg-dir))
> -        ;; FIXME: not the friendliest, but simple.
> -        (require 'info)
> -        (info-initialize)
> -        (add-to-list 'Info-directory-list pkg-dir))
> +        (with-eval-after-load 'info
> +          (info-initialize)
> +          (add-to-list 'Info-directory-list pkg-dir)))
>        (push name package-activated-list)
>        ;; Don't return nil.
>        t)))

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

Hi Philip,

Thanks for the comment.

I'm tracking the startup performance issue on my local, and found the
line 896, after change to the patched lines, it works for me several
weeks without regression.

I'll check other lines you mentioned, thanks

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 62767@debbugs.gnu.org,  eliz@gnu.org
> Date: Fri, 14 Apr 2023 20:07:54 +0000
> 
> lin Sun <sunlin7@yahoo.com> writes:
> 
> > Hi,
> >
> > The package.el will always require the entire `info.el` on its package
> > activating function.
> > Using the `with-eval-after-load` to arrange the work can avoid the
> > unnecessary loading the `info` package.
> 
> Note that this is not the only place where info is required.  If this is
> an acceptable solution (which I am not sure since `with-eval-after-load'
> is something I usually encounter in user configurations), then we should
> think about consistently addressing the issue:

What exactly are the problems with requiring info.el?  We require
packages that define some function in gazillion other places, so what
makes this one case special?

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: 62767@debbugs.gnu.org,  eliz@gnu.org
>> Date: Fri, 14 Apr 2023 20:07:54 +0000
>> 
>> lin Sun <sunlin7@yahoo.com> writes:
>> 
>> > Hi,
>> >
>> > The package.el will always require the entire `info.el` on its package
>> > activating function.
>> > Using the `with-eval-after-load` to arrange the work can avoid the
>> > unnecessary loading the `info` package.
>> 
>> Note that this is not the only place where info is required.  If this is
>> an acceptable solution (which I am not sure since `with-eval-after-load'
>> is something I usually encounter in user configurations), then we should
>> think about consistently addressing the issue:
>
> What exactly are the problems with requiring info.el?  We require
> packages that define some function in gazillion other places, so what
> makes this one case special?

Some people optimise the startup time of Emacs and want to avoid loading
files that are not necessary for Emacs to start-up.

-- 
Philip Kaludercic

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: sunlin7@yahoo.com,  62767@debbugs.gnu.org
> Date: Sat, 15 Apr 2023 10:43:21 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk@posteo.net>
> >> Cc: 62767@debbugs.gnu.org,  eliz@gnu.org
> >> Date: Fri, 14 Apr 2023 20:07:54 +0000
> >> 
> >> lin Sun <sunlin7@yahoo.com> writes:
> >> 
> >> > Hi,
> >> >
> >> > The package.el will always require the entire `info.el` on its package
> >> > activating function.
> >> > Using the `with-eval-after-load` to arrange the work can avoid the
> >> > unnecessary loading the `info` package.
> >> 
> >> Note that this is not the only place where info is required.  If this is
> >> an acceptable solution (which I am not sure since `with-eval-after-load'
> >> is something I usually encounter in user configurations), then we should
> >> think about consistently addressing the issue:
> >
> > What exactly are the problems with requiring info.el?  We require
> > packages that define some function in gazillion other places, so what
> > makes this one case special?
> 
> Some people optimise the startup time of Emacs and want to avoid loading
> files that are not necessary for Emacs to start-up.

And this is the only place in Emacs where we have (require 'FOO) and
FOO is not necessary for Emacs to start up?  I'd be surprised.

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

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

Hi Philip,

I had researched these three calls with the "require 'info", and
updated the patch for better performance.
The 1 th and 3rd can be rewritten with the new form, but the 2nd one
is necessary, and should not be changed.
Please help review the new attached patch file. Thanks.

Hi Eli,
> And this is the only place in Emacs where we have (require 'FOO) and
> FOO is not necessary for Emacs to start up?
My configuration will load the package.el at startup, so I tracked the
initial packages, and found the "require `info" will take ~1 seconds
on my old PC.
I didn't pay attention to other packages, if I found a heavy one, I'll
post it on the mail list.

[-- Attachment #2: 0001-lisp-emacs-lisp-package.el-better-way-to-load-the-in.patch --]
[-- Type: text/x-patch, Size: 2181 bytes --]

From 91a4bdbccc6f3210f43f46891077c57afd128830 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] *lisp/emacs-lisp/package.el: better way to load the info
 package

---
 lisp/emacs-lisp/package.el | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index f92afe56b7..98153a49c7 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -892,10 +892,9 @@ package-activate-1
         (add-to-list 'load-path (directory-file-name pkg-dir)))
       ;; Add info node.
       (when (file-exists-p (expand-file-name "dir" pkg-dir))
-        ;; FIXME: not the friendliest, but simple.
-        (require 'info)
-        (info-initialize)
-        (add-to-list 'Info-directory-list pkg-dir))
+        (with-eval-after-load 'info
+          (info-initialize)
+          (add-to-list 'Info-directory-list pkg-dir)))
       (push name package-activated-list)
       ;; Don't return nil.
       t)))
@@ -4427,7 +4426,7 @@ package-quickstart-refresh
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
   (package-initialize 'no-activate)
-  (require 'info)
+  (require 'info) ; insure Info-directory-list avaliable for package-activate
   (let ((package--quickstart-pkgs ())
         ;; Pretend we haven't activated anything yet!
         (package-activated-list ())
@@ -4472,10 +4471,10 @@ package-quickstart-refresh
           (current-buffer))
       (let ((info-dirs (butlast Info-directory-list)))
         (when info-dirs
-          (pp `(progn (require 'info)
-                      (info-initialize)
-                      (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
+          (pp `(with-eval-after-load 'info
+                 (info-initialize)
+                 (setq Info-directory-list
+                       (append ',info-dirs Info-directory-list)))
               (current-buffer))))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
-- 
2.20.5

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

lin Sun via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes:

> Hi Philip,
>
> I had researched these three calls with the "require 'info", and
> updated the patch for better performance.
> The 1 th and 3rd can be rewritten with the new form, but the 2nd one
> is necessary, and should not be changed.

> @@ -4427,7 +4426,7 @@ package-quickstart-refresh
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
>    (package-initialize 'no-activate)
> -  (require 'info)
> +  (require 'info) ; insure Info-directory-list avaliable for package-activate

"insure" -> "ensure"?

>    (let ((package--quickstart-pkgs ())
>          ;; Pretend we haven't activated anything yet!
>          (package-activated-list ())
> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>            (current-buffer))
>        (let ((info-dirs (butlast Info-directory-list)))
>          (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> +          (pp `(with-eval-after-load 'info
> +                 (info-initialize)
> +                 (setq Info-directory-list
> +                       (append ',info-dirs Info-directory-list)))
>                (current-buffer))))

I did some testing, and noticed that `with-eval-after-load' returns nil
to its caller when the feature is not loaded.  That is, when `info' is
not already loaded, you are effectively calling (pp nil).  This is not
feature-equivalent to the original code, so this might require the
`require' form.

-- 
Best,


RY

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

Ruijie Yu <ruijie@netyu.xyz> writes:

> lin Sun via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes:
>
>>    (let ((package--quickstart-pkgs ())
>>          ;; Pretend we haven't activated anything yet!
>>          (package-activated-list ())
>> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>>            (current-buffer))
>>        (let ((info-dirs (butlast Info-directory-list)))
>>          (when info-dirs
>> -          (pp `(progn (require 'info)
>> -                      (info-initialize)
>> -                      (setq Info-directory-list
>> -                            (append ',info-dirs Info-directory-list)))
>> +          (pp `(with-eval-after-load 'info
>> +                 (info-initialize)
>> +                 (setq Info-directory-list
>> +                       (append ',info-dirs Info-directory-list)))
>>                (current-buffer))))
>
> I did some testing, and noticed that `with-eval-after-load' returns nil
> to its caller when the feature is not loaded.  That is, when `info' is
> not already loaded, you are effectively calling (pp nil).  This is not
> feature-equivalent to the original code, so this might require the
> `require' form.

Sorry, I misread the code.  Please ignore this portion of my previous
email.

-- 
Best,


RY

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Philip Kaludercic]

lin Sun <sunlin7@yahoo.com> writes:

> Hi Philip,
>
> I had researched these three calls with the "require 'info", and
> updated the patch for better performance.
> The 1 th and 3rd can be rewritten with the new form, 

OK, then we still have to find out if these changes are safe.  As
mentioned before, I hesitate in using `with-eval-after-load', probably
because of the complicated history of `eval-after-load'.  I suppose that
Stefan might know more (as it is due to him that I am careful here), so
I have added him to the thread.

>                                                      but the 2nd one
> is necessary, and should not be changed.

How did you come to the conclusion about the second one?

> Please help review the new attached patch file. Thanks.
>
> Hi Eli,
>> And this is the only place in Emacs where we have (require 'FOO) and
>> FOO is not necessary for Emacs to start up?
> My configuration will load the package.el at startup, so I tracked the
> initial packages, and found the "require `info" will take ~1 seconds
> on my old PC.
> I didn't pay attention to other packages, if I found a heavy one, I'll
> post it on the mail list.
>
> From 91a4bdbccc6f3210f43f46891077c57afd128830 Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7@hotmail.com>
> Date: Tue, 11 Apr 2023 00:00:13 +0000
> Subject: [PATCH] *lisp/emacs-lisp/package.el: better way to load the info
>  package
>
> ---
>  lisp/emacs-lisp/package.el | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index f92afe56b7..98153a49c7 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -892,10 +892,9 @@ package-activate-1
>          (add-to-list 'load-path (directory-file-name pkg-dir)))
>        ;; Add info node.
>        (when (file-exists-p (expand-file-name "dir" pkg-dir))
> -        ;; FIXME: not the friendliest, but simple.
> -        (require 'info)
> -        (info-initialize)
> -        (add-to-list 'Info-directory-list pkg-dir))
> +        (with-eval-after-load 'info
> +          (info-initialize)
> +          (add-to-list 'Info-directory-list pkg-dir)))
>        (push name package-activated-list)
>        ;; Don't return nil.
>        t)))
> @@ -4427,7 +4426,7 @@ package-quickstart-refresh
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
>    (package-initialize 'no-activate)
> -  (require 'info)
> +  (require 'info) ; insure Info-directory-list avaliable for package-activate
>    (let ((package--quickstart-pkgs ())
>          ;; Pretend we haven't activated anything yet!
>          (package-activated-list ())
> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>            (current-buffer))
>        (let ((info-dirs (butlast Info-directory-list)))
>          (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> +          (pp `(with-eval-after-load 'info
> +                 (info-initialize)
> +                 (setq Info-directory-list
> +                       (append ',info-dirs Info-directory-list)))
>                (current-buffer))))
>        ;; Use `\s' instead of a space character, so this code chunk is not
>        ;; mistaken for an actual file-local section of package.el.

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

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

Hi Philip,

> How did you come to the conclusion about the second one?
The 2nd one is necessary because the following lines will use the full
`Info-directory-list` to print its values into the
package-quickstart.el, whose value was modified by the function
`package-activate'.
If I comment out or use the `eval-after-load` on the 2nd one, the
`Info-directory-list` will be empty, and missed in the
package-quickstart.el file.

Hi Ruijie,

> "insure" -> "ensure"?
You're right.
I had fixed my typo in the patch file (only typo in comment), and
attached the new one, please help review again. Thanks

[-- Attachment #2: 0001-lisp-emacs-lisp-package.el-better-way-to-load-the-in.patch --]
[-- Type: text/x-patch, Size: 2181 bytes --]

From 91a4bdbccc6f3210f43f46891077c57afd128830 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] *lisp/emacs-lisp/package.el: better way to load the info
 package

---
 lisp/emacs-lisp/package.el | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index f92afe56b7..98153a49c7 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -892,10 +892,9 @@ package-activate-1
         (add-to-list 'load-path (directory-file-name pkg-dir)))
       ;; Add info node.
       (when (file-exists-p (expand-file-name "dir" pkg-dir))
-        ;; FIXME: not the friendliest, but simple.
-        (require 'info)
-        (info-initialize)
-        (add-to-list 'Info-directory-list pkg-dir))
+        (with-eval-after-load 'info
+          (info-initialize)
+          (add-to-list 'Info-directory-list pkg-dir)))
       (push name package-activated-list)
       ;; Don't return nil.
       t)))
@@ -4427,7 +4426,7 @@ package-quickstart-refresh
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
   (package-initialize 'no-activate)
-  (require 'info)
+  (require 'info) ; ensure Info-directory-list available for package-activate
   (let ((package--quickstart-pkgs ())
         ;; Pretend we haven't activated anything yet!
         (package-activated-list ())
@@ -4472,10 +4471,10 @@ package-quickstart-refresh
           (current-buffer))
       (let ((info-dirs (butlast Info-directory-list)))
         (when info-dirs
-          (pp `(progn (require 'info)
-                      (info-initialize)
-                      (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
+          (pp `(with-eval-after-load 'info
+                 (info-initialize)
+                 (setq Info-directory-list
+                       (append ',info-dirs Info-directory-list)))
               (current-buffer))))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
-- 
2.20.5

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Philip Kaludercic]

lin Sun <sunlin7@yahoo.com> writes:

> Hi Philip,
>
>> How did you come to the conclusion about the second one?
> The 2nd one is necessary because the following lines will use the full
> `Info-directory-list` to print its values into the
> package-quickstart.el, whose value was modified by the function
> `package-activate'.
> If I comment out or use the `eval-after-load` on the 2nd one, the
> `Info-directory-list` will be empty, and missed in the
> package-quickstart.el file.

OK, I get it now.  I was confused because it seemed as though
`Info-directory-list' could just be bound and declared using a defvar
before the function, but if we do not load info then the
with-eval-after-load blocks in `package-activate-1' that are invoked for
the side effect of manipulating `Info-directory-list' do not get
evaluated, which is why we see that the value is empty.

But as the function `package-quickstart-refresh' is not invoked on
startup, but as `package-quickstart' says

    This requires the use of `package-quickstart-refresh' every time the
    activations need to be changed, such as when `package-load-list' is
    modified.

This should be fine.

> Hi Ruijie,
>
>> "insure" -> "ensure"?
> You're right.
> I had fixed my typo in the patch file (only typo in comment), and
> attached the new one, please help review again. Thanks

Again, assuming that there is no issue with `with-eval-after-load', the
code should be fine.  The commit message looks a bit distorted though?
Did you generate it using C-c C-w?  I get this template when I apply
your commit:

* lisp/emacs-lisp/package.el (package-activate-1):
(package-quickstart-refresh):

Also, it would make sense to motivate the changes in more detail.  What
do the changes improve?

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Mon, 17 Apr 2023 15:27:09 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 62767@debbugs.gnu.org, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>
> 
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -892,10 +892,9 @@ package-activate-1
>          (add-to-list 'load-path (directory-file-name pkg-dir)))
>        ;; Add info node.
>        (when (file-exists-p (expand-file-name "dir" pkg-dir))
> -        ;; FIXME: not the friendliest, but simple.
> -        (require 'info)
> -        (info-initialize)
> -        (add-to-list 'Info-directory-list pkg-dir))
> +        (with-eval-after-load 'info
> +          (info-initialize)
> +          (add-to-list 'Info-directory-list pkg-dir)))
>        (push name package-activated-list)
>        ;; Don't return nil.
>        t)))
> @@ -4427,7 +4426,7 @@ package-quickstart-refresh
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
>    (package-initialize 'no-activate)
> -  (require 'info)
> +  (require 'info) ; ensure Info-directory-list available for package-activate
>    (let ((package--quickstart-pkgs ())
>          ;; Pretend we haven't activated anything yet!
>          (package-activated-list ())
> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>            (current-buffer))
>        (let ((info-dirs (butlast Info-directory-list)))
>          (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> +          (pp `(with-eval-after-load 'info
> +                 (info-initialize)
> +                 (setq Info-directory-list
> +                       (append ',info-dirs Info-directory-list)))
>                (current-buffer))))
>        ;; Use `\s' instead of a space character, so this code chunk is not
>        ;; mistaken for an actual file-local section of package.el.
> -- 
> 2.20.5

I don't know yet whether I'm okay with this change, but if this does
go in, this tricky/unusual code must have comments explaining why on
Earth we do it this way, and only with info.el.

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

> OK, then we still have to find out if these changes are safe.  As
> mentioned before, I hesitate in using `with-eval-after-load', probably
> because of the complicated history of `eval-after-load'.  I suppose that
> Stefan might know more (as it is due to him that I am careful here), so
> I have added him to the thread.

The problem with eval-after-load is not the history but the fact that
it has various corner-case caveats.

(with-eval-after-load "compile" ...) for example makes it unclear
exactly when it should be executed, e.g.:

   (load "compile")                          => yes clearly.
   (load "compile.el")                       => probably, yes.
   (load "srecode/compile.el")               => probably not.
   (load "/foo/bar/lisp/compile.el")         => hmm... yeah, I think so.
   (load "/foo/bar/lisp/srecode/compile.el") => hmm... nah, maybe not.

The next game is to try and guess what the code does, to see when it
matches expectations.

The use (with-eval-after-load 'info ...) is much better in this respect
because it's not linked to the file name but to the feature name, which
is precise.

But there's still the issue of exactly when this is executed.  Should it
be executed just the next time the file is loaded (usually what's
wanted), or every other time after that as well (what actually happens)?

Also the implementation for the good case where we provide a feature
name rather than a file name is a bit roundabout/fiddly.  This is an
internal problem, so any bug there can be fixed, thus it's probably not
a good reason not to use it, but it still makes me prefer to avoid it :-)

I wish there was an easier way to add to `Info-directory-list`.
That variable is set in a fairly delicate manner, so I think Eli would
be better placed to come up with a better long term solution.
Maybe we should just add a (preloaded) `Info-extra-directory-list`?

In the mean time, Lin Sun's patch is probably "as good as it gets" if
you want to load `info.el` lazily (which I obviously didn't bother to do
when I wrote that code :-)


        Stefan

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

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

Hi Eli,

I had researched the `info.el', and found the variable
`Info-default-directory-list' is a more proper variable than the
original `Info-directory-list', which is an autoload customer
variable.

Then the patch can be simpler than the previous one, I attached the
new patch, it avoided to require 'info. I verified it on my local.

Please help review the change, thanks !

[-- Attachment #2: 0001-lisp-emacs-lisp-package.el-avoid-to-load-the-entire-.patch --]
[-- Type: text/x-patch, Size: 2860 bytes --]

From 1b93c5469c98254e1c6ab7494272401c5e97ddf5 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] *lisp/emacs-lisp/package.el: avoid to load the entire info
 package

---
 lisp/emacs-lisp/package.el | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ffa6272dd1..fb26cff463 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -819,7 +819,7 @@ package--autoloads-file-name
    (format "%s-autoloads" (package-desc-name pkg-desc))
    (package-desc-dir pkg-desc)))
 
-(defvar Info-directory-list)
+(defvar Info-default-directory-list)
 (declare-function info-initialize "info" ())
 
 (defvar package--quickstart-pkgs t
@@ -907,10 +907,7 @@ package-activate-1
         (add-to-list 'load-path (directory-file-name pkg-dir)))
       ;; Add info node.
       (when (file-exists-p (expand-file-name "dir" pkg-dir))
-        ;; FIXME: not the friendliest, but simple.
-        (require 'info)
-        (info-initialize)
-        (add-to-list 'Info-directory-list pkg-dir))
+        (add-to-list 'Info-default-directory-list pkg-dir))
       (push name package-activated-list)
       ;; Don't return nil.
       t)))
@@ -4458,7 +4455,6 @@ package-quickstart-refresh
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
   (package-initialize 'no-activate)
-  (require 'info)
   (let ((package--quickstart-pkgs ())
         ;; Pretend we haven't activated anything yet!
         (package-activated-list ())
@@ -4468,7 +4464,7 @@ package-quickstart-refresh
         ;; aren't truncated.
         (print-length nil)
         (print-level nil)
-        (Info-directory-list '("")))
+        (Info-default-directory-list '("")))
     (dolist (elt package-alist)
       (condition-case err
           (package-activate (car elt))
@@ -4501,12 +4497,11 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
+      (let ((info-dirs (butlast Info-default-directory-list)))
         (when info-dirs
-          (pp `(progn (require 'info)
-                      (info-initialize)
-                      (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
+          (pp `(defvar Info-default-directory-list) (current-buffer))
+          (pp `(setq Info-default-directory-list
+                     (append ',info-dirs Info-default-directory-list))
               (current-buffer))))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
-- 
2.20.5

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Wed, 19 Apr 2023 05:11:03 +0000
> Cc: Philip Kaludercic <philipk@posteo.net>, Eli Zaretskii <eliz@gnu.org>, 62767@debbugs.gnu.org
> 
> I had researched the `info.el', and found the variable
> `Info-default-directory-list' is a more proper variable than the
> original `Info-directory-list', which is an autoload customer
> variable.
> 
> Then the patch can be simpler than the previous one, I attached the
> new patch, it avoided to require 'info. I verified it on my local.

Thanks, but we cannot possibly change Info-default-directory-list
behind the user's back.  Info-default-directory-list is a user option,
so Emacs itself cannot modify its value.

Moreover, Info-default-directory-list is considered only once, when
info.el is loaded and initialized for the first time.  Whereas
package.el can be used for installing a package in the middle of a
running Emacs session, when info.el was already loaded, and so
modifying Info-default-directory-list will not have any effect until
the next restart of Emacs.  Do we require users to restart Emacs after
installing a package via package.el?

Going back to the original problem, AFAIU you wanted to avoid loading
info.el at startup, because that makes startup slower, is that right?
Then how about adding a new variable, Info-packages-directory-list,
say, to which package.el will add directories of the installed
packages, and info.el will use when it first loads?  Would that solve
the problem?

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

On Thu, 20 Apr 2023 12:12:29 +0300 Eli Zaretskii <eliz <at> gnu.org> wrote:
> Thanks, but we cannot possibly change Info-default-directory-list
> behind the user's back.  Info-default-directory-list is a user option,
> so Emacs itself cannot modify its value.
>
> Moreover, Info-default-directory-list is considered only once, when
> info.el is loaded and initialized for the first time.  Whereas
> package.el can be used for installing a package in the middle of a
> running Emacs session, when info.el was already loaded, and so
> modifying Info-default-directory-list will not have any effect until
> the next restart of Emacs.  Do we require users to restart Emacs after
> installing a package via package.el?
>
> Going back to the original problem, AFAIU you wanted to avoid loading
> info.el at startup, because that makes startup slower, is that right?
> Then how about adding a new variable, Info-packages-directory-list,
> say, to which package.el will add directories of the installed
> packages, and info.el will use when it first loads?  Would that solve
> the problem?
Totally, Info-default-directory-list is a customer variable, and it is
considered only once, not the perfect one.
And yes, the original motivation is to avoid loading info.el at startup.
With your comment, I'm going to search better way to avoid loading
info.el, thanks !

Best Regards
Lin

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

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

Hi Eli, Stefan,

I am experimenting with a different approach to avoid the loading of
the entire info.el file using package.el.
Instead, I am adding the directories to the `Info-directory-list',
which will be utilized by other functions responsible for loading the
specific info files.

Please help review the patch I attached. Thanks.

Best regards
Lin

[-- Attachment #2: 0001-avoid-to-load-the-entire-info-package.patch --]
[-- Type: text/x-patch, Size: 4637 bytes --]

From efd3cbdb5ce5d7eeb2601557775ff3f93e86d476 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] avoid to load the entire info package

*lisp/emacs-lisp/package.el: don't require info package
*lisp/info.el: use explicit mark `Info--initialized'
---
 lisp/emacs-lisp/package.el | 17 ++++++-----------
 lisp/info.el               | 33 ++++++++++++++++++---------------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 293c1c39ca..70e995e3dc 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -907,9 +907,6 @@ package-activate-1
         (add-to-list 'load-path (directory-file-name pkg-dir)))
       ;; Add info node.
       (when (file-exists-p (expand-file-name "dir" pkg-dir))
-        ;; FIXME: not the friendliest, but simple.
-        (require 'info)
-        (info-initialize)
         (add-to-list 'Info-directory-list pkg-dir))
       (push name package-activated-list)
       ;; Don't return nil.
@@ -4478,7 +4475,6 @@ package-quickstart-refresh
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
   (package-initialize 'no-activate)
-  (require 'info)
   (let ((package--quickstart-pkgs ())
         ;; Pretend we haven't activated anything yet!
         (package-activated-list ())
@@ -4521,13 +4517,12 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
-        (when info-dirs
-          (pp `(progn (require 'info)
-                      (info-initialize)
-                      (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
-              (current-buffer))))
+      (when-let ((info-dirs (butlast Info-directory-list)))
+        (pp `(defvar Info-directory-list) (current-buffer))
+        (pp `(setq Info-directory-list
+                   (delete-dups
+                    (append ',info-dirs Info-directory-list)))
+            (current-buffer)))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
       (insert "\f
diff --git a/lisp/info.el b/lisp/info.el
index 035dff66e7..8f53240dab 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -400,6 +400,9 @@ Info-virtual-nodes
 (defvar-local Info-current-node-virtual nil
   "Non-nil if the current Info node is virtual.")
 
+(defvar Info--initialized nil
+  "Non-nil if `info-initialize' has been run.")
+
 (defun Info-virtual-file-p (filename)
   "Check if Info file FILENAME is virtual."
   (Info-virtual-fun 'find-file filename nil))
@@ -700,29 +703,29 @@ Info-default-dirs
 
 (defun info-initialize ()
   "Initialize `Info-directory-list', if that hasn't been done yet."
-  (unless Info-directory-list
+  (unless Info--initialized
     (let ((path (getenv "INFOPATH"))
 	  (sep (regexp-quote path-separator)))
-      (setq Info-directory-list
-	    (prune-directory-list
-	     (if path
-		 (if (string-match-p (concat sep "\\'") path)
-		     (append (split-string (substring path 0 -1) sep)
-			     (Info-default-dirs))
-		   (split-string path sep))
-	       (Info-default-dirs))))
+      (dolist (dir
+	       (prune-directory-list
+	        (if path
+		    (if (string-match-p (concat sep "\\'") path)
+		        (append (split-string (substring path 0 -1) sep)
+			        (Info-default-dirs))
+		      (split-string path sep))
+	          (Info-default-dirs))))
+        (add-to-list 'Info-directory-list dir))
       ;; For a self-contained (ie relocatable) NS build, AFAICS we
       ;; always want the included info directory to be at the head of
       ;; the search path, unless it's already in INFOPATH somewhere.
       ;; It's at the head of Info-default-directory-list,
       ;; but there's no way to get it at the head of Info-directory-list
       ;; except by doing it here.
-      (and path
-	   (featurep 'ns)
-	   (let ((dir (expand-file-name "../info" data-directory)))
-	     (and (file-directory-p dir)
-		  (not (member dir (split-string path ":" t)))
-		  (push dir Info-directory-list)))))))
+      (when (and path (featurep 'ns))
+        (when-let* ((dir (expand-file-name "../info" data-directory))
+                    ((file-directory-p dir)))
+	  (add-to-list 'Info-directory-list dir)))))
+  (setq Info--initialized t))
 
 ;;;###autoload
 (defun info-other-window (&optional file-or-node buffer)
-- 
2.20.5

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[lin Sun]

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

> Hi Eli,
> I am experimenting with a different approach to avoid the loading of
> the entire info.el file using package.el.
> Instead, I am adding the directories to the `Info-directory-list',
> which will be utilized by other functions responsible for loading the
> specific info files.
> Please help review the patch I attached. Thanks.

[-- Attachment #2: 0001-avoid-to-load-the-entire-info-package-V2.patch --]
[-- Type: application/x-patch, Size: 4950 bytes --]

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Wed, 24 May 2023 02:53:08 +0000
> 
> > I am experimenting with a different approach to avoid the loading of
> > the entire info.el file using package.el.
> > Instead, I am adding the directories to the `Info-directory-list',
> > which will be utilized by other functions responsible for loading the
> > specific info files.
> > Please help review the patch I attached. Thanks.

Could you please describe how this makes sure Info-directory-list will
be set to a correct value, in the various scenarios that are relevant?

In particular, Info-directory-list is not the right variable to tweak
here, as it is computed by info.el.  I think we need a separate
variable.

> -      (let ((info-dirs (butlast Info-directory-list)))
> -        (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> -              (current-buffer))))
> +      (when-let ((info-dirs (butlast Info-directory-list)))
> +        (pp `(defvar Info-directory-list '()) (current-buffer))
> +        (pp `(setq Info-directory-list
> +                   (delete-dups
> +                    (append ',info-dirs Info-directory-list)))
> +            (current-buffer)))

Using 'append' here could cause duplicate directories in
Info-directory-list.

> +(defvar Info--initialized nil
> +  "Non-nil if `info-initialize' has been run.")

There's no need to capitalize the first letter of the name of an
internal variable.  We capitalize 'I' in "Info" so that it would be
easier to type info.el commands with completion (other commands that
begin with 'i' use lower-case 'i').  This is not a factor for internal
variables.

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

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

On Wed, May 24, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> ...
> Could you please describe how this makes sure Info-directory-list will
> be set to a correct value, in the various scenarios that are relevant?
>
> In particular, Info-directory-list is not the right variable to tweak
> here, as it is computed by info.el.  I think we need a separate
> variable.
In the original function `info-initialize', it will rely on the value
`Info-directory-list' to indicate function initialization and parse
the "INFOPATH" or get system info dirs as the initialized value.

The bug comes for: package.el will add several paths into
`Info-directory-list', that maybe lead function `info-initialize'
return without parsing the "INFOPATH" or getting the system info dires
as initial value.

So package.el required the entire `info.el` and called function
`info-initialize' to initialize the value first (by parsing the
"INFOPATH" or get system info dir), then the package.el modify the
`Info-directory-list'.

The patch  will allow users to set the `Info-directory-list' before
calling function `info-initialize'; when info.el use the
`Info-directory-list' variable to read plain *.info file, will call
function `info-initialize' to add system info dirs.

Two functions `Info-insert-dir' and `Info-find-file', will use
(get/read) the value of `Info-directory-list' to read the plain *.info
files.
In the patch file, both functions will call the function
`info-initialize' at their entry to ensure the `Info-directory-list'
is initialized with the "INFOPATH" env string.
Users will just insert/remove dirs from/to `Info-directory-list', but
won't use the list to read/get plain *.info files.
So users can define the `Info-directory-list' and modify its value on
their elisp files.

> > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > +        (pp `(setq Info-directory-list
> > +                   (delete-dups
> > +                    (append ',info-dirs Info-directory-list)))
> > +            (current-buffer)))
>
> Using 'append' here could cause duplicate directories in
> Info-directory-list.
The `delete-dups' will avoid that.

> > +(defvar Info--initialized nil
> > +  "Non-nil if `info-initialize' has been run.")
> There's no need to capitalize the first letter of the name of an
> internal variable.  We capitalize 'I' in "Info" so that it would be
> easier to type info.el commands with completion (other commands that
> begin with 'i' use lower-case 'i').  This is not a factor for internal
> variables.
Very helpful to understand the naming rules. And rename it to
`info--initialized', new patch attached, please review again.

Best Regards
Lin

[-- Attachment #2: 0001-avoid-to-load-the-entire-info-package-V3.patch --]
[-- Type: text/x-patch, Size: 4950 bytes --]

From a17f389ce6af7600714d0cadf925a9bcd5a6ccb1 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] avoid to load the entire info package

*lisp/emacs-lisp/package.el: don't require info package
*lisp/info.el: use explicit mark `Info--initialized'
---
 lisp/emacs-lisp/package.el | 17 ++++++-----------
 lisp/info.el               | 34 +++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 293c1c39ca..46de846b80 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -907,9 +907,6 @@ package-activate-1
         (add-to-list 'load-path (directory-file-name pkg-dir)))
       ;; Add info node.
       (when (file-exists-p (expand-file-name "dir" pkg-dir))
-        ;; FIXME: not the friendliest, but simple.
-        (require 'info)
-        (info-initialize)
         (add-to-list 'Info-directory-list pkg-dir))
       (push name package-activated-list)
       ;; Don't return nil.
@@ -4478,7 +4475,6 @@ package-quickstart-refresh
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
   (package-initialize 'no-activate)
-  (require 'info)
   (let ((package--quickstart-pkgs ())
         ;; Pretend we haven't activated anything yet!
         (package-activated-list ())
@@ -4521,13 +4517,12 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
-        (when info-dirs
-          (pp `(progn (require 'info)
-                      (info-initialize)
-                      (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
-              (current-buffer))))
+      (when-let ((info-dirs (butlast Info-directory-list)))
+        (pp `(defvar Info-directory-list '()) (current-buffer))
+        (pp `(setq Info-directory-list
+                   (delete-dups
+                    (append ',info-dirs Info-directory-list)))
+            (current-buffer)))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
       (insert "\f
diff --git a/lisp/info.el b/lisp/info.el
index 035dff66e7..631c35140b 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -400,6 +400,9 @@ Info-virtual-nodes
 (defvar-local Info-current-node-virtual nil
   "Non-nil if the current Info node is virtual.")
 
+(defvar info--initialized nil
+  "Non-nil if `info-initialize' has been run.")
+
 (defun Info-virtual-file-p (filename)
   "Check if Info file FILENAME is virtual."
   (Info-virtual-fun 'find-file filename nil))
@@ -700,29 +703,29 @@ Info-default-dirs
 
 (defun info-initialize ()
   "Initialize `Info-directory-list', if that hasn't been done yet."
-  (unless Info-directory-list
+  (unless info--initialized
     (let ((path (getenv "INFOPATH"))
 	  (sep (regexp-quote path-separator)))
-      (setq Info-directory-list
-	    (prune-directory-list
-	     (if path
-		 (if (string-match-p (concat sep "\\'") path)
-		     (append (split-string (substring path 0 -1) sep)
-			     (Info-default-dirs))
-		   (split-string path sep))
-	       (Info-default-dirs))))
+      (dolist (dir
+	       (prune-directory-list
+	        (if path
+		    (if (string-match-p (concat sep "\\'") path)
+		        (append (split-string (substring path 0 -1) sep)
+			        (Info-default-dirs))
+		      (split-string path sep))
+	          (Info-default-dirs))))
+        (add-to-list 'Info-directory-list dir))
       ;; For a self-contained (ie relocatable) NS build, AFAICS we
       ;; always want the included info directory to be at the head of
       ;; the search path, unless it's already in INFOPATH somewhere.
       ;; It's at the head of Info-default-directory-list,
       ;; but there's no way to get it at the head of Info-directory-list
       ;; except by doing it here.
-      (and path
-	   (featurep 'ns)
-	   (let ((dir (expand-file-name "../info" data-directory)))
-	     (and (file-directory-p dir)
-		  (not (member dir (split-string path ":" t)))
-		  (push dir Info-directory-list)))))))
+      (when (and path (featurep 'ns))
+        (when-let* ((dir (expand-file-name "../info" data-directory))
+                    ((file-directory-p dir)))
+	  (add-to-list 'Info-directory-list dir)))))
+  (setq info--initialized t))
 
 ;;;###autoload
 (defun info-other-window (&optional file-or-node buffer)
@@ -1279,6 +1282,7 @@ Info-dir-file-name
 ;; default-directory to the first directory we actually get any text
 ;; from.
 (defun Info-insert-dir ()
+  (info-initialize)
   (if (and Info-dir-contents Info-dir-file-attributes
 	   ;; Verify that none of the files we used has changed
 	   ;; since we used it.
-- 
2.20.5

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Thu, 25 May 2023 00:46:58 +0000
> Cc: 62767@debbugs.gnu.org
> 
> On Wed, May 24, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > ...
> > Could you please describe how this makes sure Info-directory-list will
> > be set to a correct value, in the various scenarios that are relevant?
> >
> > In particular, Info-directory-list is not the right variable to tweak
> > here, as it is computed by info.el.  I think we need a separate
> > variable.
> In the original function `info-initialize', it will rely on the value
> `Info-directory-list' to indicate function initialization and parse
> the "INFOPATH" or get system info dirs as the initialized value.
> 
> The bug comes for: package.el will add several paths into
> `Info-directory-list', that maybe lead function `info-initialize'
> return without parsing the "INFOPATH" or getting the system info dires
> as initial value.
> 
> So package.el required the entire `info.el` and called function
> `info-initialize' to initialize the value first (by parsing the
> "INFOPATH" or get system info dir), then the package.el modify the
> `Info-directory-list'.
> 
> The patch  will allow users to set the `Info-directory-list' before
> calling function `info-initialize'; when info.el use the
> `Info-directory-list' variable to read plain *.info file, will call
> function `info-initialize' to add system info dirs.
> 
> Two functions `Info-insert-dir' and `Info-find-file', will use
> (get/read) the value of `Info-directory-list' to read the plain *.info
> files.
> In the patch file, both functions will call the function
> `info-initialize' at their entry to ensure the `Info-directory-list'
> is initialized with the "INFOPATH" env string.
> Users will just insert/remove dirs from/to `Info-directory-list', but
> won't use the list to read/get plain *.info files.
> So users can define the `Info-directory-list' and modify its value on
> their elisp files.

I understand that part.  But I don't think it's TRT to have users or
Lisp programs outside of info.el manipulate Info-directory-list and
mutate it.  Instead, I think we should have a package.el-specific
directory list of Info files, say, package-info-directory-list, and we
should arrange for info.el to look in those directories _before_ it
looks in the directories mentioned by Info-directory-list.  Then
there'll be no need to play with info-initialize and the variable we
use to indicate info-initialize was already called, and (more
importantly), no code outside of info.el will mess with
Info-directory-list.  And package.el will add directories to this new
variable.

OK?

> > > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > > +        (pp `(setq Info-directory-list
> > > +                   (delete-dups
> > > +                    (append ',info-dirs Info-directory-list)))
> > > +            (current-buffer)))
> >
> > Using 'append' here could cause duplicate directories in
> > Info-directory-list.
> The `delete-dups' will avoid that.

Yes, but why not use add-to-list in the first place?

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

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

On Fri, May 26, 2023 at 9:37 AM Eli Zaretskii <eliz@gnu.org> wrote:
>  But I don't think it's TRT to have users or
> Lisp programs outside of info.el manipulate Info-directory-list and
> mutate it.  Instead, I think we should have a package.el-specific
> directory list of Info files, say, package-info-directory-list, and we
> should arrange for info.el to look in those directories _before_ it
> looks in the directories mentioned by Info-directory-list.  Then
> there'll be no need to play with info-initialize and the variable we
> use to indicate info-initialize was already called, and (more
> importantly), no code outside of info.el will mess with
> Info-directory-list.  And package.el will add directories to this new
> variable.
>
> OK?
Totally understand your concern.
The proposed mechanism will request info.el to reserve a variable that
allows customer change outside of info.el, then info.el reads the
variable and proceeds the paths before the info.el major function. So
adding a new variable package-info-directory-list or using the current
Info-directory-list are nearly equal, except that the new name is for
package.el special.

> > > > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > > > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > > > +        (pp `(setq Info-directory-list
> > > > +                   (delete-dups
> > > > +                    (append ',info-dirs Info-directory-list)))
> > > > +            (current-buffer)))
> > >
> > > Using 'append' here could cause duplicate directories in
> > > Info-directory-list.
> > The `delete-dups' will avoid that.
>
> Yes, but why not use add-to-list in the first place?
`add-to-list' will be a good choice here.
The code is following its previous code lines, almost same format,
paste as follow,
>      (pp `(defvar package-activated-list) (current-buffer))
>      (pp `(setq package-activated-list
>                 (delete-dups
>                  (append ',(mapcar #'package-desc-name package--quickstart-pkgs)

bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

[Eli Zaretskii]

> From: lin Sun <sunlin7@yahoo.com>
> Date: Tue, 30 May 2023 03:56:00 +0000
> Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 62767@debbugs.gnu.org
> 
> The proposed mechanism will request info.el to reserve a variable that
> allows customer change outside of info.el, then info.el reads the
> variable and proceeds the paths before the info.el major function. So
> adding a new variable package-info-directory-list or using the current
> Info-directory-list are nearly equal, except that the new name is for
> package.el special.

Yes, that's the idea.  This way, we keep info.el and package.el very
loosely coupled, as they should be, and each one of them can apply its
own logic to its variable.

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

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

Hi Eli,

There is a build error after I pull the last emacs-29 branch, tried
build on CentOS 7.
> use of undeclared identifier emacs 'SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION'
The macro SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION is defined since
SQLite 3.13 (2016 May).
CentOS 7 will end by 2024 June 30, so this patch is still useful.

Best Regards
Lin

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

[lin Sun]

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

Attachments

[-- Attachment #2: 0001-More-fix-for-loading-SQLite-extensions.patch --]
[-- Type: text/x-patch, Size: 1760 bytes --]

From 332e9c3c2253b40584ba76de4e3644204acbfcdc Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 23 May 2023 00:00:12 +0000
Subject: [PATCH] More fix for loading SQLite extensions

* src/sqlite.c (sqlite3_db_config): Load from the dylib.
(Fsqlite_load_extension): rely on the SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION
existence.
---
 src/sqlite.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/sqlite.c b/src/sqlite.c
index 77ce61ba65..363a52b317 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -722,6 +722,7 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
       if (strlen (*allow) < strlen (name)
 	  && !strncmp (*allow, name, strlen (*allow))
 	  && (!strcmp (name + strlen (*allow), ".so")
+	      || !strcmp (name + strlen (*allow), ".dylib")
 	      || !strcasecmp (name + strlen (*allow), ".dll")))
 	{
 	  do_allow = true;
@@ -737,14 +738,18 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
      "leak" this outside this function.  */
   sqlite3 *sdb = XSQLITE (db)->db;
   char *ext_fn = SSDATA (ENCODE_FILE (Fexpand_file_name (module, Qnil)));
+#ifdef SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION
   /* Temporarily enable loading extensions via the C API.  */
   int result = sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1,
 				  NULL);
   if (result == SQLITE_OK)
+#endif
     {
-      result = sqlite3_load_extension (sdb, ext_fn, NULL, NULL);
+      int result = sqlite3_load_extension (sdb, ext_fn, NULL, NULL);
       /* Disable loading extensions via C API.  */
+#ifdef SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION
       sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 0, NULL);
+#endif
       if (result == SQLITE_OK)
 	return Qt;
     }
-- 
2.20.5

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

[Eli Zaretskii]

> From: lin Sun <sunlin7.mail@gmail.com>
> Date: Tue, 23 May 2023 04:36:08 +0000
> 
> Attachments

Thanks.  But in that case this is not the right patch.  If SQLite3
cannot support enabling and disabling loading of extensions, it means
we cannot support extension loading with such old SQLite3 versions.

So I've now installed a slightly different fix on the emacs-29 branch;
please try it.

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

[lin Sun]

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

Hi Eli,

It works for me with the latest change on sqlite.c. Thank you !
And now I want add one line to support loading extensions with *.dylib,
Please help review the patch . Thanks

[-- Attachment #2: 0001-src-sqlite.c-sqlite-load-extension-Also-load-from-th.patch --]
[-- Type: text/x-patch, Size: 766 bytes --]

From a8ea552ea2dc34d4a22dded208b8bfd81e206e1f Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 23 May 2023 13:25:39 +0000
Subject: [PATCH] * src/sqlite.c (sqlite-load-extension): Also load from the
 dylib.

---
 src/sqlite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/sqlite.c b/src/sqlite.c
index 852e3746ef..c863659222 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -733,6 +733,7 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
       if (strlen (*allow) < strlen (name)
 	  && !strncmp (*allow, name, strlen (*allow))
 	  && (!strcmp (name + strlen (*allow), ".so")
+	      || !strcmp (name + strlen (*allow), ".dylib")
 	      || !strcasecmp (name + strlen (*allow), ".dll")))
 	{
 	  do_allow = true;
-- 
2.20.5

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

[Eli Zaretskii]

> From: lin Sun <sunlin7.mail@gmail.com>
> Date: Tue, 23 May 2023 13:40:39 +0000
> Cc: 63653@debbugs.gnu.org
> 
> And now I want add one line to support loading extensions with *.dylib,
> Please help review the patch . Thanks

Thanks, I installed something similar.

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

[lin Sun]

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

Great, see the changes. Thanks!

On Tue, May 23, 2023, 07:54 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: lin Sun <sunlin7.mail@gmail.com>
> > Date: Tue, 23 May 2023 13:40:39 +0000
> > Cc: 63653@debbugs.gnu.org
> >
> > And now I want add one line to support loading extensions with *.dylib,
> > Please help review the patch . Thanks
>
> Thanks, I installed something similar.
>

[-- Attachment #2: Type: text/html, Size: 852 bytes --]

bug#63653: 29.0.91; [PATCH] More fix for loading SQLite extensions

[lin Sun]

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

Sorry for no PATH file in my previous mail.
Here's the path file.

[-- Attachment #2: 0001-More-fix-for-loading-SQLite-extensions.patch --]
[-- Type: text/x-patch, Size: 1760 bytes --]

From 332e9c3c2253b40584ba76de4e3644204acbfcdc Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 23 May 2023 00:00:12 +0000
Subject: [PATCH] More fix for loading SQLite extensions

* src/sqlite.c (sqlite3_db_config): Load from the dylib.
(Fsqlite_load_extension): rely on the SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION
existence.
---
 src/sqlite.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/sqlite.c b/src/sqlite.c
index 77ce61ba65..363a52b317 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -722,6 +722,7 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
       if (strlen (*allow) < strlen (name)
 	  && !strncmp (*allow, name, strlen (*allow))
 	  && (!strcmp (name + strlen (*allow), ".so")
+	      || !strcmp (name + strlen (*allow), ".dylib")
 	      || !strcasecmp (name + strlen (*allow), ".dll")))
 	{
 	  do_allow = true;
@@ -737,14 +738,18 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
      "leak" this outside this function.  */
   sqlite3 *sdb = XSQLITE (db)->db;
   char *ext_fn = SSDATA (ENCODE_FILE (Fexpand_file_name (module, Qnil)));
+#ifdef SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION
   /* Temporarily enable loading extensions via the C API.  */
   int result = sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1,
 				  NULL);
   if (result == SQLITE_OK)
+#endif
     {
-      result = sqlite3_load_extension (sdb, ext_fn, NULL, NULL);
+      int result = sqlite3_load_extension (sdb, ext_fn, NULL, NULL);
       /* Disable loading extensions via C API.  */
+#ifdef SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION
       sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 0, NULL);
+#endif
       if (result == SQLITE_OK)
 	return Qt;
     }
-- 
2.20.5

bug#64386: 29.0.91; [PATCH] *src/emacs.c: a comma for elements of usage_message

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

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

Hi,

It missed a comma in the array "usage_message". It won't hurt the
usage, but the lint tool (clang-tidy) will give a warning.

Please help review that patch. Thanks.

[-- Attachment #2: 0001-src-emacs.c-a-comma-for-elements-of-usage_message.patch --]
[-- Type: application/x-patch, Size: 645 bytes --]

bug#64386: 29.0.91; [PATCH] *src/emacs.c: a comma for elements of usage_message

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

Lin Sun <sunlin7@yahoo.com> writes:

> Hi,
>
> It missed a comma in the array "usage_message". It won't hurt the
> usage, but the lint tool (clang-tidy) will give a warning.
>
> Please help review that patch. Thanks.

Thanks, but we prefer to avoid purely stylistic changes, deferring them
until they can be made in conjunction with other nearby changes to code.

bug#64386: 29.0.91; [PATCH] *src/emacs.c: a comma for elements of usage_message

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

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

Agree, please feel free to close this ticket, thanks

On Fri, Jun 30, 2023, 18:34 Po Lu <luangruo@yahoo.com> wrote:

> Lin Sun <sunlin7@yahoo.com> writes:
>
> > Hi,
> >
> > It missed a comma in the array "usage_message". It won't hurt the
> > usage, but the lint tool (clang-tidy) will give a warning.
> >
> > Please help review that patch. Thanks.
>
> Thanks, but we prefer to avoid purely stylistic changes, deferring them
> until they can be made in conjunction with other nearby changes to code.
>

[-- Attachment #2: Type: text/html, Size: 907 bytes --]

bug#64386: 29.0.91; [PATCH] *src/emacs.c: a comma for elements of usage_message

[Stefan Kangas]

> Agree, please feel free to close this ticket, thanks

Done, thanks.

bug#64386: 29.0.91; [PATCH] *src/emacs.c: a comma for elements of usage_message

[Eli Zaretskii]

> From: Lin Sun <sunlin7@yahoo.com>
> Date: Fri, 30 Jun 2023 14:55:14 -0700
> 
> It missed a comma in the array "usage_message". It won't hurt the
> usage, but the lint tool (clang-tidy) will give a warning.

Which warning do you get?  And why do you think the comma is missing?

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

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

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

Hi,

In a EWW buffer, the user can use the 'eww-open-in-new-buffer' or "M-RET" to
open a URL in a new buffer and focus on the new buffer.

Sometimes people want to open the URLs in the background first and read them
later, so here is the 'eww-open-in-new-buffer-background' function,
binding to "C-<return>".

Example: using the eww to open the hack news and browse the topics,
open some topics in background with "C-<return>", and read them later.

Please help review the patch. Thanks.

Best Regards
Lin

[-- Attachment #2: 0001-lisp-net-eww.el-new-function-eww-open-in-new-buffer-.patch --]
[-- Type: text/x-patch, Size: 1631 bytes --]

From 23e8580726cffc6883ad7f553414f257c263e2ba Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Wed, 16 Aug 2023 01:00:07 +0000
Subject: [PATCH] *lisp/net/eww.el: new function
 `eww-open-in-new-buffer-backgroud'

---
 lisp/net/eww.el | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index cb73926f46..3e9ad51f10 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -561,6 +561,12 @@ eww-open-in-new-buffer
           (eww-mode)
           (eww (if (consp url) (car url) url)))))))
 
+(defun eww-open-in-new-buffer-background ()
+  "Fetch link at point in a new background EWW buffer."
+  (interactive)
+  (save-window-excursion
+    (call-interactively #'eww-open-in-new-buffer)))
+
 (defun eww-html-p (content-type)
   "Return non-nil if CONTENT-TYPE designates an HTML content type.
 Currently this means either text/html or application/xhtml+xml."
@@ -1073,6 +1079,7 @@ eww-mode-map
   "g" #'eww-reload             ;FIXME: revert-buffer-function instead!
   "G" #'eww
   "M-RET" #'eww-open-in-new-buffer
+  "C-<return>" #'eww-open-in-new-buffer-background
   "TAB" #'shr-next-link
   "C-M-i" #'shr-previous-link
   "<backtab>" #'shr-previous-link
@@ -1112,6 +1119,7 @@ eww-mode-map
           ["Close browser" quit-window t]
           ["Reload" eww-reload t]
           ["Follow URL in new buffer" eww-open-in-new-buffer]
+          ["Follow URL in background" eww-open-in-new-buffer-background]
           ["Back to previous page" eww-back-url
            :active (not (zerop (length eww-history)))]
           ["Forward to next page" eww-forward-url
-- 
2.20.5

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Eli Zaretskii]

> Date: Wed, 16 Aug 2023 19:39:30 +0000
> From:  Lin Sun via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> In a EWW buffer, the user can use the 'eww-open-in-new-buffer' or "M-RET" to
> open a URL in a new buffer and focus on the new buffer.
> 
> Sometimes people want to open the URLs in the background first and read them
> later, so here is the 'eww-open-in-new-buffer-background' function,
> binding to "C-<return>".
> 
> Example: using the eww to open the hack news and browse the topics,
> open some topics in background with "C-<return>", and read them later.

"Background" is a problematic word for this, since that's not what you
mean: you mean not to select the window displaying the URL.

And I don't think we need a separate command for that: how about doing
this in eww-open-in-new-buffer instead, if the user invokes it with a
prefix argument C-u?

And finally, what happens if the following condition, tested by
eww-open-in-new-buffer, is fulfilled:

      (when (or (eq eww-browse-url-new-window-is-tab t)
                (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
                     tab-bar-mode))

In this case, AFAIU the URL is opened in a new tab.

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

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

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

Hi Eli,
> "Background" is a problematic word for this, since that's not what you
> mean: you mean not to select the window displaying the URL.
>
> And I don't think we need a separate command for that: how about doing
> this in eww-open-in-new-buffer instead, if the user invokes it with a
> prefix argument C-u?
>
> And finally, what happens if the following condition, tested by
> eww-open-in-new-buffer, is fulfilled:
>
>       (when (or (eq eww-browse-url-new-window-is-tab t)
>                 (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
>                      tab-bar-mode))
>
> In this case, AFAIU the URL is opened in a new tab.

Thank you for the comment, and agree with you, so I rewrote the
function `eww-open-in-new-buffer' with "C-u" support (also with an
optional argument "url"), then there is no "background" description.

I also tested the new patch with tab-bar-mode on and off, it works for
both scenarios.

Please help review again. Thanks.

Best regards
Lin

[-- Attachment #2: 0001-lisp-net-eww.el-eww-open-in-new-buffer-able-to-stay-.patch --]
[-- Type: text/x-patch, Size: 2696 bytes --]

From 317019bcc1f659bf71dc20c1998dc8b4623b0fe1 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Wed, 16 Aug 2023 01:00:07 +0000
Subject: [PATCH] *lisp/net/eww.el: `eww-open-in-new-buffer' able to stay on
 current buffer

---
 lisp/net/eww.el | 50 +++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index cb73926f46..23b4fe3117 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -542,24 +542,38 @@ eww-search-words
           (call-interactively #'eww)))
     (call-interactively #'eww)))
 
-(defun eww-open-in-new-buffer ()
-  "Fetch link at point in a new EWW buffer."
-  (interactive)
-  (let ((url (eww-suggested-uris)))
-    (if (null url) (user-error "No link at point")
-      (when (or (eq eww-browse-url-new-window-is-tab t)
-                (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
-                     tab-bar-mode))
-        (let ((tab-bar-new-tab-choice t))
-          (tab-new)))
-      ;; clone useful to keep history, but
-      ;; should not clone from non-eww buffer
-      (with-current-buffer
-          (if (eq major-mode 'eww-mode) (clone-buffer)
-            (generate-new-buffer "*eww*"))
-        (unless (equal url (eww-current-url))
-          (eww-mode)
-          (eww (if (consp url) (car url) url)))))))
+(defun eww--open-url-in-new-buffer (url)
+  "Open the URL in a new EWW buffer."
+  ;; clone useful to keep history, but
+  ;; should not clone from non-eww buffer
+  (with-current-buffer
+      (if (eq major-mode 'eww-mode) (clone-buffer)
+        (generate-new-buffer "*eww*"))
+    (unless (equal url (eww-current-url))
+      (eww-mode)
+      (eww (if (consp url) (car url) url)))))
+
+(defun eww-open-in-new-buffer (stay &optional url)
+  "Fetch URL in a new EWW buffer.
+
+If the STAY is not `nil', the forcus will stay on current buffer.
+
+If the URL is `nil', it will try `eww-suggested-uris' under current cursor."
+  (interactive "P")
+  (if-let ((url (or url (eww-suggested-uris))))
+      (if (or (eq eww-browse-url-new-window-is-tab t)
+              (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
+                   tab-bar-mode))
+          (let ((tab-bar-new-tab-choice t))
+            (tab-new)
+            (eww--open-url-in-new-buffer url)
+            (when stay
+              (tab-bar-switch-to-prev-tab)))
+
+        (if stay
+            (save-window-excursion (eww--open-url-in-new-buffer url))
+          (eww--open-url-in-new-buffer url)))
+    (user-error "No avaliable link")))
 
 (defun eww-html-p (content-type)
   "Return non-nil if CONTENT-TYPE designates an HTML content type.
-- 
2.20.5

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Eli Zaretskii]

> From: Lin Sun <sunlin7@yahoo.com>
> Date: Thu, 17 Aug 2023 22:51:04 +0000
> Cc: 65346@debbugs.gnu.org
> 
> Thank you for the comment, and agree with you, so I rewrote the
> function `eww-open-in-new-buffer' with "C-u" support (also with an
> optional argument "url"), then there is no "background" description.
> 
> I also tested the new patch with tab-bar-mode on and off, it works for
> both scenarios.
> 
> Please help review again. Thanks.

Thanks.  However, the way you implemented my suggestion is not what I
had in mind.

> -(defun eww-open-in-new-buffer ()
> -  "Fetch link at point in a new EWW buffer."
> -  (interactive)
> -  (let ((url (eww-suggested-uris)))
> -    (if (null url) (user-error "No link at point")
> -      (when (or (eq eww-browse-url-new-window-is-tab t)
> -                (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
> -                     tab-bar-mode))
> -        (let ((tab-bar-new-tab-choice t))
> -          (tab-new)))
> -      ;; clone useful to keep history, but
> -      ;; should not clone from non-eww buffer
> -      (with-current-buffer
> -          (if (eq major-mode 'eww-mode) (clone-buffer)
> -            (generate-new-buffer "*eww*"))
> -        (unless (equal url (eww-current-url))
> -          (eww-mode)
> -          (eww (if (consp url) (car url) url)))))))
> +(defun eww--open-url-in-new-buffer (url)
> +  "Open the URL in a new EWW buffer."
> +  ;; clone useful to keep history, but
> +  ;; should not clone from non-eww buffer
> +  (with-current-buffer
> +      (if (eq major-mode 'eww-mode) (clone-buffer)
> +        (generate-new-buffer "*eww*"))
> +    (unless (equal url (eww-current-url))
> +      (eww-mode)
> +      (eww (if (consp url) (car url) url)))))
> +
> +(defun eww-open-in-new-buffer (stay &optional url)
> +  "Fetch URL in a new EWW buffer.

This changes the API of a public function in incompatible ways, which
we try very hard not to do.  Instead, the signature or
eww-open-in-new-buffer should be like this:

  (defun eww-open-in-new-buffer (&optional no-select)

and the new NO-SELECT argument should be set non-nil by C-u.

> +    (user-error "No avaliable link")))

This also changes the text of the error message in this case.  Was
that really necessary?  The original text was more accurate, I think.

Also, this new feature needs a NEWS entry and a suitable addition to
the EWW manual.

Thanks.

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

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

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

On Fri, Aug 18, 2023 at 5:52 AM Eli Zaretskii <eliz@gnu.org> wrote:
> This changes the API of a public function in incompatible ways, which
> we try very hard not to do.  Instead, the signature or
> eww-open-in-new-buffer should be like this:
>
>   (defun eww-open-in-new-buffer (&optional no-select)
>
> and the new NO-SELECT argument should be set non-nil by C-u.
>
> > +    (user-error "No avaliable link")))
>
> This also changes the text of the error message in this case.  Was
> that really necessary?  The original text was more accurate, I think.
>
> Also, this new feature needs a NEWS entry and a suitable addition to
> the EWW manual.

The attached patch included changes for the function, error message,
NEWS and EWW manual.
Please help review it.

Thanks. Regards

[-- Attachment #2: 0001-lisp-net-eww.el-eww-open-in-new-buffer-able-to-stay-.patch --]
[-- Type: text/x-patch, Size: 3813 bytes --]

From 2fb95d761d4d53e8b34253d89fd06ea267f3eb0d Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Wed, 16 Aug 2023 01:00:07 +0000
Subject: [PATCH] *lisp/net/eww.el: `eww-open-in-new-buffer' able to stay on
 current buffer

---
 doc/misc/eww.texi |  3 ++-
 etc/NEWS          |  5 +++++
 lisp/net/eww.el   | 49 ++++++++++++++++++++++++++++++-----------------
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/doc/misc/eww.texi b/doc/misc/eww.texi
index b67624af9f..2181355a57 100644
--- a/doc/misc/eww.texi
+++ b/doc/misc/eww.texi
@@ -137,7 +137,8 @@ Basics
 ``tab'' in other browsers.  When @code{global-tab-line-mode} is
 enabled, this buffer is displayed in the tab on the window tab line.
 When @code{tab-bar-mode} is enabled, a new tab is created on the frame
-tab bar.
+tab bar.  If the prefix key @kbd{C-u} is avaliable, it will stay on
+current buffer.
 
 @findex eww-readable
 @kindex R
diff --git a/etc/NEWS b/etc/NEWS
index 808b599672..13aedee2d5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -452,6 +452,11 @@ the kill ring.  Alternate links are optional metadata that HTML pages
 use for linking to their alternative representations, such as
 translated versions or associated RSS feeds.
 
++++
+*** 'eww-open-in-new-buffer' support prefix key "C-u" to stay current buffer.
+The command accept the prefix key "C-u" to open the url in a new
+buffer but stay in current buffer, won't jump to the new buffer.
+
 ** go-ts-mode
 
 +++
diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index cb73926f46..f2ed60f172 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -542,24 +542,37 @@ eww-search-words
           (call-interactively #'eww)))
     (call-interactively #'eww)))
 
-(defun eww-open-in-new-buffer ()
-  "Fetch link at point in a new EWW buffer."
-  (interactive)
-  (let ((url (eww-suggested-uris)))
-    (if (null url) (user-error "No link at point")
-      (when (or (eq eww-browse-url-new-window-is-tab t)
-                (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
-                     tab-bar-mode))
-        (let ((tab-bar-new-tab-choice t))
-          (tab-new)))
-      ;; clone useful to keep history, but
-      ;; should not clone from non-eww buffer
-      (with-current-buffer
-          (if (eq major-mode 'eww-mode) (clone-buffer)
-            (generate-new-buffer "*eww*"))
-        (unless (equal url (eww-current-url))
-          (eww-mode)
-          (eww (if (consp url) (car url) url)))))))
+(defun eww--open-url-in-new-buffer (url)
+  "Open the URL in a new EWW buffer."
+  ;; clone useful to keep history, but
+  ;; should not clone from non-eww buffer
+  (with-current-buffer
+      (if (eq major-mode 'eww-mode) (clone-buffer)
+        (generate-new-buffer "*eww*"))
+    (unless (equal url (eww-current-url))
+      (eww-mode)
+      (eww (if (consp url) (car url) url)))))
+
+(defun eww-open-in-new-buffer (&optional no-select url)
+  "Fetch URL in a new EWW buffer.
+
+If the NO-SELECT is not `nil', the forcus will stay on current buffer.
+
+If the URL is `nil', it will try `eww-suggested-uris' under current cursor."
+  (interactive "P")
+  (if-let ((url (or url (eww-suggested-uris))))
+      (if (or (eq eww-browse-url-new-window-is-tab t)
+              (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
+                   tab-bar-mode))
+          (let ((tab-bar-new-tab-choice t))
+            (tab-new)
+            (eww--open-url-in-new-buffer url)
+            (when no-select
+              (tab-bar-switch-to-prev-tab)))
+        (if no-select
+            (save-window-excursion (eww--open-url-in-new-buffer url))
+          (eww--open-url-in-new-buffer url)))
+    (user-error "No link at point")))
 
 (defun eww-html-p (content-type)
   "Return non-nil if CONTENT-TYPE designates an HTML content type.
-- 
2.20.5

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Eli Zaretskii]

> From: Lin Sun <sunlin7@yahoo.com>
> Date: Fri, 18 Aug 2023 19:03:08 +0000
> Cc: 65346@debbugs.gnu.org
> 
> On Fri, Aug 18, 2023 at 5:52 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > This changes the API of a public function in incompatible ways, which
> > we try very hard not to do.  Instead, the signature or
> > eww-open-in-new-buffer should be like this:
> >
> >   (defun eww-open-in-new-buffer (&optional no-select)
> >
> > and the new NO-SELECT argument should be set non-nil by C-u.
> >
> > > +    (user-error "No avaliable link")))
> >
> > This also changes the text of the error message in this case.  Was
> > that really necessary?  The original text was more accurate, I think.
> >
> > Also, this new feature needs a NEWS entry and a suitable addition to
> > the EWW manual.
> 
> The attached patch included changes for the function, error message,
> NEWS and EWW manual.
> Please help review it.

Thanks, installed on master, and closing the bug.

Please in the future include a more detailed log of the changes in the
commit log message, and please follow our conventions for that as
described in CONTRIBUTE.  I did it for you this time, but it required
quite a lot of manual work.

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

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

Hi Eli,

On Sun, Aug 20, 2023 at 8:49 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> Thanks, installed on master, and closing the bug.
>
> Please in the future include a more detailed log of the changes in the
> commit log message, and please follow our conventions for that as
> described in CONTRIBUTE.  I did it for you this time, but it required
> quite a lot of manual work.

Thank you so much, really appreciate it !  Best Regards

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Juri Linkov]

> +(defun eww-open-in-new-buffer (&optional no-select url)
> +  "Fetch URL in a new EWW buffer.
> +
> +If the NO-SELECT is not `nil', the forcus will stay on current buffer.
> +
> +If the URL is `nil', it will try `eww-suggested-uris' under current cursor."
> +  (interactive "P")
> +  (if-let ((url (or url (eww-suggested-uris))))
> +      (if (or (eq eww-browse-url-new-window-is-tab t)
> +              (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
> +                   tab-bar-mode))
> +          (let ((tab-bar-new-tab-choice t))
> +            (tab-new)
> +            (eww--open-url-in-new-buffer url)
> +            (when no-select
> +              (tab-bar-switch-to-prev-tab)))

tab-bar-switch-to-prev-tab is a wrong function, please use
tab-bar-switch-to-recent-tab instead.

Also please add a new command to open a tab in background.
Then new command could be bound to the standard RET modifier
used in all web browsers to open a tab in background.

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  65346@debbugs.gnu.org
> Date: Sun, 20 Aug 2023 19:44:03 +0300
> 
> > +(defun eww-open-in-new-buffer (&optional no-select url)
> > +  "Fetch URL in a new EWW buffer.
> > +
> > +If the NO-SELECT is not `nil', the forcus will stay on current buffer.
> > +
> > +If the URL is `nil', it will try `eww-suggested-uris' under current cursor."
> > +  (interactive "P")
> > +  (if-let ((url (or url (eww-suggested-uris))))
> > +      (if (or (eq eww-browse-url-new-window-is-tab t)
> > +              (and (eq eww-browse-url-new-window-is-tab 'tab-bar)
> > +                   tab-bar-mode))
> > +          (let ((tab-bar-new-tab-choice t))
> > +            (tab-new)
> > +            (eww--open-url-in-new-buffer url)
> > +            (when no-select
> > +              (tab-bar-switch-to-prev-tab)))
> 
> tab-bar-switch-to-prev-tab is a wrong function, please use
> tab-bar-switch-to-recent-tab instead.

Thanks, done.

> Also please add a new command to open a tab in background.
> Then new command could be bound to the standard RET modifier
> used in all web browsers to open a tab in background.

I didn't do that; patches welcome.

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> "Background" is a problematic word for this, since that's not what you
> mean: you mean not to select the window displaying the URL.

Right - but isn't that the wording known from other browsers?

> And I don't think we need a separate command for that: how about doing
> this in eww-open-in-new-buffer instead, if the user invokes it with a
> prefix argument C-u?

That's not very convenient, though.  How about adding an option that
toggles the meaning of the prefix arg?

Michael.

bug#65346: 30.0.50; *lisp/net/eww.el: new function 'eww-open-in-new-buffer-background'

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Lin Sun <sunlin7@yahoo.com>,  65346@debbugs.gnu.org
> Date: Fri, 18 Aug 2023 04:15:57 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > "Background" is a problematic word for this, since that's not what you
> > mean: you mean not to select the window displaying the URL.
> 
> Right - but isn't that the wording known from other browsers?

Not AFAIK, no.  And in Emacs, "background" has several other meanings,
which could confuse.

> > And I don't think we need a separate command for that: how about doing
> > this in eww-open-in-new-buffer instead, if the user invokes it with a
> > prefix argument C-u?
> 
> That's not very convenient, though.  How about adding an option that
> toggles the meaning of the prefix arg?

Fine by me.

bug#66710: 29.0.1; [PATCH] *doc/misc/gnus.texi: Fix unmatched quote in gnus doc

[Lin Sun]

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

Hi,

An unmatched quote in the doc/misc/gnus.texi causes its following part
to be displayed as a string.

This patch will fix the issue.

Please help to apply it. Thanks.

Best Regards
Lin

[-- Attachment #2: 0001-doc-misc-gnus.texi-Fix-unmatched-quote-in-gnus-doc.patch --]
[-- Type: text/x-patch, Size: 820 bytes --]

From d06c61cfb1e37135cbbd772f888c128bc871754e Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Mon, 23 Oct 2023 05:00:01 +0000
Subject: [PATCH] *doc/misc/gnus.texi: Fix unmatched quote in gnus doc

---
 doc/misc/gnus.texi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/misc/gnus.texi b/doc/misc/gnus.texi
index 86819b8707..216bbed496 100644
--- a/doc/misc/gnus.texi
+++ b/doc/misc/gnus.texi
@@ -3216,7 +3216,7 @@ Group Parameters
 @end example
 
 To generate tests for multiple email-addresses use a group parameter
-like @code{(sieve address "sender" ("name@@one.org" else@@two.org"))}.
+like @code{(sieve address "sender" ("name@@one.org" "else@@two.org"))}.
 When generating a sieve script (@pxref{Sieve Commands}) Sieve code
 like the following is generated:
 
-- 
2.20.5

bug#66710: 29.0.1; [PATCH] *doc/misc/gnus.texi: Fix unmatched quote in gnus doc

[Eli Zaretskii]

close 66710 29.2
thanks

> From: Lin Sun <sunlin7.mail@gmail.com>
> Date: Mon, 23 Oct 2023 17:15:18 +0000
> Cc: "bug-gnu-emacs@gnu.org" <bug-gnu-emacs@gnu.org>, Eli Zaretskii <eliz@gnu.org>
> 
> An unmatched quote in the doc/misc/gnus.texi causes its following part
> to be displayed as a string.
> 
> This patch will fix the issue.
> 
> Please help to apply it. Thanks.

Thanks, I installed this on the emacs-29 branch.

Please note that the commit log message was not entirely according to
our standards; please see how I modified it, and try in the future to
use this style (see CONTRIBUTE for more details).

bug#62767: 29.0.1; [PATCH] *doc/misc/gnus.texi: Fix unmatched quote in gnus doc

[Lin Sun]

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

Appreciate, thank you so much!

Send from my Android phone

On Mon, Oct 23, 2023, 11:57 Eli Zaretskii <eliz@gnu.org> wrote:

> close 66710 29.2
> thanks
>
> > From: Lin Sun <sunlin7.mail@gmail.com>
> > Date: Mon, 23 Oct 2023 17:15:18 +0000
> > Cc: "bug-gnu-emacs@gnu.org" <bug-gnu-emacs@gnu.org>, Eli Zaretskii <
> eliz@gnu.org>
> >
> > An unmatched quote in the doc/misc/gnus.texi causes its following part
> > to be displayed as a string.
> >
> > This patch will fix the issue.
> >
> > Please help to apply it. Thanks.
>
> Thanks, I installed this on the emacs-29 branch.
>
> Please note that the commit log message was not entirely according to
> our standards; please see how I modified it, and try in the future to
> use this style (see CONTRIBUTE for more details).
>

[-- Attachment #2: Type: text/html, Size: 1486 bytes --]

bug#60209: 29.0.50; [PATCH] lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el

[Stefan Kangas]

> I got a build error with last commit 2b1fdbffcb595bcd72fa9aa3db674c6985042bcb,
>
> error ("Version must be a string")
>   error("Version must be a string")
>   version-to-list(29.1)
>   version<(29.1 "19.29")
>   #f(compiled-function (e1 e2) #<bytecode 0x7353c27725409c8>)((29.1
> ruby-mode) ("19.29" time-stamp))
>
> This patch will fix the error. Please merge it, thanks.

Thanks for the patch, and sorry for the very late reply.

This issue is already fixed on master, so I'm closing this bug.