bug#65575: [PATCH 3/3] gnu: emacs: Reload subdirs.el files in 'guix-emacs-autoload-packages'.

2023-08-29 Thread Liliana Marie Prikler
Am Montag, dem 28.08.2023 um 11:20 -0400 schrieb Maxim Cournoyer:
> > e.g. 
> > (defun guix-emacs-autoload-packages ( reload)
> >   "..."
> >   (interactive "P")
> >   (when reload (mapc #'load-file (guix-emacs--subdirs-files)))
> >   ...)
> > 
> > WDYT?
> 
> The reason for avoiding loading the subdirs.el files on the first
> call is just an optimization, since it would at this time duplicate
> work already done by Emacs itself when it first executes.  This
> shouldn't fail; I've now employed the same 'noerror strategy as used
> for autoloads to ensure that.
> 
> There's one edge case I've just thought though, which is if a user
> invoked emacs with the documented '--no-site-file' option disabling
> loading autoloads; this would cause guix-emacs-autoload-packages-
> called to be nil.
> 
> To balance between making things both convenient and flexible, I've
> preserved the tracking but also added the reload override you
> suggested.  Let me know what you think.
Assuming convenience equates to not needing to type C-u, we can also
achieve that without tracking:

(defun guix-emacs-autoload-packages ( noexpand)
  "Autoload Emacs packages found in EMACSLOADPATH.

'Autoload' means to load the 'autoloads' files matching 
`guix-emacs-autoloads-regexp'.  Before doing so, expand load-path by
loading subdirs.el files found in it, unless NOEXPAND is given."
  (interactive "P")
  (unless noexpand (mapc #'load-file (guix-emacs--subdirs-files)))
  ...)

In our own init code, we should simply call it as 
  (guix-emacs-autoload-packages 'noexpand)
then, since this expansion is already done earlier by Emacs.

Cheers





bug#65575: [PATCH 3/3] gnu: emacs: Reload subdirs.el files in 'guix-emacs-autoload-packages'.

2023-08-28 Thread Maxim Cournoyer
Hi Liliana,

Liliana Marie Prikler  writes:

> Hi Maxim,
>
> Am Montag, dem 28.08.2023 um 01:11 -0400 schrieb Maxim Cournoyer:
>> This fixes a regression introduced with 79cfe30f3 ("build-system:
>> emacs: Use subdirectories again.") which caused the
>> 'guix-emacs-autoload-packages' to no
>> longer be able to autoload all packages.
>>
>> * gnu/packages/aux-files/emacs/guix-emacs.el
>> (guix-emacs-autoload-packages-called): New variable.
>> (guix-emacs-autoload-packages): Reload subdirs.el files an all but
>> the first call of this procedure.
>> * doc/guix.texi (Application Setup): Document that
>> 'guix-emacs-autoload-packages' can be invoked interactively to auto-
>> reload newly installed Emacs packages.
>>
>> ---
> Thank you for looking at this.  I agree that even if we don't want to
> generally load new packages into existing Emacs processes for the
> breakages they might induce, not being able to do so at all is also not
> a great option.  However, I don't think that tracking is the right
> solution here, see below.

[...]

>> diff --git a/gnu/packages/aux-files/emacs/guix-emacs.el
>> b/gnu/packages/aux-files/emacs/guix-emacs.el
>> index ed0c913163..b4a4fd1d91 100644
>> --- a/gnu/packages/aux-files/emacs/guix-emacs.el
>> +++ b/gnu/packages/aux-files/emacs/guix-emacs.el
>> @@ -54,6 +54,9 @@ The files in the list do not have extensions (.el,
>> .elc)."
>>  (expand-file-name "subdirs.el" dir))
>>    (guix-emacs--non-core-load-path
>>  
>> +(defvar guix-emacs-autoload-packages-called nil
>> +  "True if `guix-emacs-autoload-packages' was already run.")
>> +
>>  ;;;###autoload
>>  (defun guix-emacs-autoload-packages ()
>>    "Autoload Emacs packages found in EMACSLOADPATH.
>> @@ -61,6 +64,15 @@ The files in the list do not have extensions (.el,
>> .elc)."
>>  'Autoload' means to load the 'autoloads' files matching
>>  `guix-emacs-autoloads-regexp'."
>>    (interactive)
>> +  ;; Reload the subdirs.el files such as the one generated by the
>> Guix profile
>> +  ;; hook, so that newly installed Emacs packages located under
>> +  ;; sub-directories are put on the load-path without having to
>> restart Emacs.
>> +  (if guix-emacs-autoload-packages-called
>> +  (progn
>> +    (mapc #'load-file (guix-emacs--subdirs-files))
>> +    (setq load-path (delete-dups load-path)))
>> +    (setq guix-emacs-autoload-packages-called t))
>> +
> Rather than keeping track of whether the function was already called
> (which would make debugging more tedious if you also have e.g. broken
> packages from another source on your EMACSLOADPATH), I think the user
> ought to signal their intent to reload the subdirs files via the
> universal argument.
>
> e.g. 
> (defun guix-emacs-autoload-packages ( reload)
>   "..."
>   (interactive "P")
>   (when reload (mapc #'load-file (guix-emacs--subdirs-files)))
>   ...)
>
> WDYT?

The reason for avoiding loading the subdirs.el files on the first call
is just an optimization, since it would at this time duplicate work
already done by Emacs itself when it first executes.  This shouldn't
fail; I've now employed the same 'noerror strategy as used for autoloads
to ensure that.

There's one edge case I've just thought though, which is if a user
invoked emacs with the documented '--no-site-file' option disabling
loading autoloads; this would cause guix-emacs-autoload-packages-called
to be nil.

To balance between making things both convenient and flexible, I've
preserved the tracking but also added the reload override you suggested.
Let me know what you think.

Thank you for the review!

-- 
Thanks,
Maxim





bug#65575: [PATCH 3/3] gnu: emacs: Reload subdirs.el files in 'guix-emacs-autoload-packages'.

2023-08-28 Thread Liliana Marie Prikler
Hi Maxim,

Am Montag, dem 28.08.2023 um 01:11 -0400 schrieb Maxim Cournoyer:
> This fixes a regression introduced with 79cfe30f3 ("build-system:
> emacs: Use subdirectories again.") which caused the
> 'guix-emacs-autoload-packages' to no
> longer be able to autoload all packages.
> 
> * gnu/packages/aux-files/emacs/guix-emacs.el
> (guix-emacs-autoload-packages-called): New variable.
> (guix-emacs-autoload-packages): Reload subdirs.el files an all but
> the first call of this procedure.
> * doc/guix.texi (Application Setup): Document that
> 'guix-emacs-autoload-packages' can be invoked interactively to auto-
> reload newly installed Emacs packages.
> 
> ---
Thank you for looking at this.  I agree that even if we don't want to
generally load new packages into existing Emacs processes for the
breakages they might induce, not being able to do so at all is also not
a great option.  However, I don't think that tracking is the right
solution here, see below.

>  doc/guix.texi  | 11 +++
>  gnu/packages/aux-files/emacs/guix-emacs.el | 12 
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index f82bb99069..66da4f9cd4 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -2167,12 +2167,15 @@ Application Setup
>  Emacs through the @env{EMACSLOADPATH} environment variable, which is
>  set when installing Emacs itself.
>  
> +@cindex guix-emacs-autoload-packages, refreshing Emacs packages
>  Additionally, autoload definitions are automatically evaluated at
> the
>  initialization of Emacs, by the Guix-specific
> -@code{guix-emacs-autoload-packages} procedure.  If, for some reason,
> you
> -want to avoid auto-loading the Emacs packages installed with Guix,
> you
> -can do so by running Emacs with the @option{--no-site-file} option
> -(@pxref{Init File,,, emacs, The GNU Emacs Manual}).
> +@code{guix-emacs-autoload-packages} procedure.  This procedure can
> be
> +interactively invoked to have newly installed Emacs packages
> discovered,
> +without having to restart Emacs.  If, for some reason, you want to
> avoid
> +auto-loading the Emacs packages installed with Guix, you can do so
> by
> +running Emacs with the @option{--no-site-file} option (@pxref{Init
> +File,,, emacs, The GNU Emacs Manual}).
>  
>  @quotation Note
>  Emacs can now compile packages natively.  Under the default
> diff --git a/gnu/packages/aux-files/emacs/guix-emacs.el
> b/gnu/packages/aux-files/emacs/guix-emacs.el
> index ed0c913163..b4a4fd1d91 100644
> --- a/gnu/packages/aux-files/emacs/guix-emacs.el
> +++ b/gnu/packages/aux-files/emacs/guix-emacs.el
> @@ -54,6 +54,9 @@ The files in the list do not have extensions (.el,
> .elc)."
>  (expand-file-name "subdirs.el" dir))
>    (guix-emacs--non-core-load-path
>  
> +(defvar guix-emacs-autoload-packages-called nil
> +  "True if `guix-emacs-autoload-packages' was already run.")
> +
>  ;;;###autoload
>  (defun guix-emacs-autoload-packages ()
>    "Autoload Emacs packages found in EMACSLOADPATH.
> @@ -61,6 +64,15 @@ The files in the list do not have extensions (.el,
> .elc)."
>  'Autoload' means to load the 'autoloads' files matching
>  `guix-emacs-autoloads-regexp'."
>    (interactive)
> +  ;; Reload the subdirs.el files such as the one generated by the
> Guix profile
> +  ;; hook, so that newly installed Emacs packages located under
> +  ;; sub-directories are put on the load-path without having to
> restart Emacs.
> +  (if guix-emacs-autoload-packages-called
> +  (progn
> +    (mapc #'load-file (guix-emacs--subdirs-files))
> +    (setq load-path (delete-dups load-path)))
> +    (setq guix-emacs-autoload-packages-called t))
> +
Rather than keeping track of whether the function was already called
(which would make debugging more tedious if you also have e.g. broken
packages from another source on your EMACSLOADPATH), I think the user
ought to signal their intent to reload the subdirs files via the
universal argument.

e.g. 
(defun guix-emacs-autoload-packages ( reload)
  "..."
  (interactive "P")
  (when reload (mapc #'load-file (guix-emacs--subdirs-files)))
  ...)

WDYT?





bug#65575: [PATCH 3/3] gnu: emacs: Reload subdirs.el files in 'guix-emacs-autoload-packages'.

2023-08-27 Thread Maxim Cournoyer
This fixes a regression introduced with 79cfe30f3 ("build-system: emacs: Use
subdirectories again.") which caused the 'guix-emacs-autoload-packages' to no
longer be able to autoload all packages.

* gnu/packages/aux-files/emacs/guix-emacs.el
(guix-emacs-autoload-packages-called): New variable.
(guix-emacs-autoload-packages): Reload subdirs.el files an all but the first
call of this procedure.
* doc/guix.texi (Application Setup): Document that
'guix-emacs-autoload-packages' can be invoked interactively to auto-reload
newly installed Emacs packages.

---

 doc/guix.texi  | 11 +++
 gnu/packages/aux-files/emacs/guix-emacs.el | 12 
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f82bb99069..66da4f9cd4 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -2167,12 +2167,15 @@ Application Setup
 Emacs through the @env{EMACSLOADPATH} environment variable, which is
 set when installing Emacs itself.
 
+@cindex guix-emacs-autoload-packages, refreshing Emacs packages
 Additionally, autoload definitions are automatically evaluated at the
 initialization of Emacs, by the Guix-specific
-@code{guix-emacs-autoload-packages} procedure.  If, for some reason, you
-want to avoid auto-loading the Emacs packages installed with Guix, you
-can do so by running Emacs with the @option{--no-site-file} option
-(@pxref{Init File,,, emacs, The GNU Emacs Manual}).
+@code{guix-emacs-autoload-packages} procedure.  This procedure can be
+interactively invoked to have newly installed Emacs packages discovered,
+without having to restart Emacs.  If, for some reason, you want to avoid
+auto-loading the Emacs packages installed with Guix, you can do so by
+running Emacs with the @option{--no-site-file} option (@pxref{Init
+File,,, emacs, The GNU Emacs Manual}).
 
 @quotation Note
 Emacs can now compile packages natively.  Under the default
diff --git a/gnu/packages/aux-files/emacs/guix-emacs.el 
b/gnu/packages/aux-files/emacs/guix-emacs.el
index ed0c913163..b4a4fd1d91 100644
--- a/gnu/packages/aux-files/emacs/guix-emacs.el
+++ b/gnu/packages/aux-files/emacs/guix-emacs.el
@@ -54,6 +54,9 @@ The files in the list do not have extensions (.el, .elc)."
 (expand-file-name "subdirs.el" dir))
   (guix-emacs--non-core-load-path
 
+(defvar guix-emacs-autoload-packages-called nil
+  "True if `guix-emacs-autoload-packages' was already run.")
+
 ;;;###autoload
 (defun guix-emacs-autoload-packages ()
   "Autoload Emacs packages found in EMACSLOADPATH.
@@ -61,6 +64,15 @@ The files in the list do not have extensions (.el, .elc)."
 'Autoload' means to load the 'autoloads' files matching
 `guix-emacs-autoloads-regexp'."
   (interactive)
+  ;; Reload the subdirs.el files such as the one generated by the Guix profile
+  ;; hook, so that newly installed Emacs packages located under
+  ;; sub-directories are put on the load-path without having to restart Emacs.
+  (if guix-emacs-autoload-packages-called
+  (progn
+(mapc #'load-file (guix-emacs--subdirs-files))
+(setq load-path (delete-dups load-path)))
+(setq guix-emacs-autoload-packages-called t))
+
   (let ((autoloads (mapcan #'guix-emacs-find-autoloads
(guix-emacs--non-core-load-path
 (mapc (lambda (f)
-- 
2.41.0