[PATCH] Various fixes for early init file changes

[PATCH] Various fixes for early init file changes

[Radon Rosborough]

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

Hello all,

Attached are three patches:

1. Fix `Info-default-directory-list' error during init. This moves the
running of `custom-reevaluate-setting' on predefined variables to
earlier during startup, so that `package-initialize' can access
`Info-default-directory-list'. Without this patch, warnings are
emitted during startup if packages with Info manuals are installed.
2. Documentation fixes for early init file changes. This clarifies in
the Emacs manual what variables need to be customized in the early
init file, and fixes a couple of docstrings in package.el that still
referred to "loading" packages rather than "making them available".
3. Warn if `package-initialize' is called twice. This causes Emacs to
emit a warning, by default, if `package-initialize' is called twice
during init. I expect this to cause people a lot of warnings, but I
think this is a good thing, since the alternative (currently) is a
pretty bad performance regression if people leave a now-superfluous
extra call to `package-initialize' in their init files. Plus, the fix
is trivial: just delete the extra call. For people who really do want
to call `package-initialize' twice for some reason, there is a new
variable `package-warn-on-reinitialization' which can be customized to
disable the warning.

I believe this addresses all the problems people have noticed with the
changes related to `package-initialize' and the early init file.
Please let me know if I missed anything. Thanks for all the feedback
and support!

Best regards,
Radon Rosborough

[-- Attachment #2: 0001-Fix-Info-default-directory-list-error-during-init.patch --]
[-- Type: application/octet-stream, Size: 2197 bytes --]

From b61d09e08302c7441e1f2301287b5fcff4303ecd Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 20:55:48 -0800
Subject: [PATCH] Fix Info-default-directory-list error during init

* lisp/startup.el (command-line): Call `custom-reevaluate-setting' on
predefined variables before loading the early init file and before
`package-initialize' is called.  This prevents
`Info-default-directory-list' from being unbound when
`package-initialize' tries to access it during startup.  See [1].

[1]: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00545.html
---
 lisp/startup.el | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index 4105c1db2d..2669342eda 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -1115,6 +1115,15 @@ command-line
     (and command-line-args
          (setcdr command-line-args args)))
 
+  ;; Re-evaluate predefined variables whose initial value depends on
+  ;; the runtime context.
+  (let (current-load-list) ; c-r-s may call defvar, and hence LOADHIST_ATTACH
+    (mapc 'custom-reevaluate-setting
+          ;; Initialize them in the same order they were loaded, in case there
+          ;; are dependencies between them.
+          (prog1 (nreverse custom-delayed-init-variables)
+            (setq custom-delayed-init-variables nil))))
+
   ;; Warn for invalid user name.
   (when init-file-user
     (if (string-match "[~/:\n]" init-file-user)
@@ -1245,15 +1254,6 @@ command-line
     (startup--setup-quote-display)
     (setq internal--text-quoting-flag t))
 
-  ;; Re-evaluate predefined variables whose initial value depends on
-  ;; the runtime context.
-  (let (current-load-list) ; c-r-s may call defvar, and hence LOADHIST_ATTACH
-    (mapc 'custom-reevaluate-setting
-          ;; Initialize them in the same order they were loaded, in case there
-          ;; are dependencies between them.
-          (prog1 (nreverse custom-delayed-init-variables)
-            (setq custom-delayed-init-variables nil))))
-
   (normal-erase-is-backspace-setup-frame)
 
   ;; Register default TTY colors for the case the terminal hasn't a
-- 
2.16.2


[-- Attachment #3: 0001-Documentation-fixes-for-early-init-file-changes.patch --]
[-- Type: application/octet-stream, Size: 4648 bytes --]

From e3b57bedf521b814d32c4fe3698b5ffc726d8e22 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 20:33:34 -0800
Subject: [PATCH] Documentation fixes for early init file changes

* doc/emacs/custom.texi (Early Init File): Add more details about
which variables must be set in the early init file rather than the
regular init file.  See [1].

* lisp/emacs-lisp/package.el (package-enable-at-startup): Update
docstring to note that packages are now made available before loading
the init file, rather than afterwards.  See [2].
(package-load-list): Refer to "making available" rather than "loading"
for packages.  See [3].

[1]: https://lists.nongnu.org/archive/html/bug-gnu-emacs/2018-02/msg00827.html
[2]: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00632.html
[3]: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00298.html
---
 doc/emacs/custom.texi      | 11 ++++++++---
 lisp/emacs-lisp/package.el | 27 ++++++++++++++++-----------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 0905ae7bb1..ca842bd1e1 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -2606,9 +2606,14 @@ Early Init File
 earlier than the normal init file is processed.  Such customizations
 can be put in the early init file, @file{~/.emacs.d/early-init.el}.
 This file is loaded before the package system is initialized, so in it
-you can customize variables that affect the initialization process,
-such as @code{package-enable-at-startup} and @code{package-load-list}.
-@xref{Package Installation}.
+you can customize variables that affect the initialization process
+(i.e. @code{package-initialize}), such as
+@code{package-enable-at-startup}, @code{package-load-list}, and
+@code{package-user-dir}.  Note that variables like
+@code{package-archives} which only affect the installation of new
+packages, and not the process of making already-installed packages
+available, may be customized in the regular init file.  @xref{Package
+Installation}.
 
   For more information on the early init file, @pxref{Init File,,,
 elisp, The Emacs Lisp Reference Manual}.
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 999e0d0752..6c307cc37c 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -161,29 +161,34 @@ package
 ;;; Customization options
 ;;;###autoload
 (defcustom package-enable-at-startup t
-  "Whether to activate installed packages when Emacs starts.
-If non-nil, packages are activated after reading the init file
-and before `after-init-hook'.  Activation is not done if
-`user-init-file' is nil (e.g. Emacs was started with \"-q\").
+  "Whether to make installed packages available when Emacs starts.
+If non-nil, packages are made available before reading the init
+file (but after reading the early init file).  This means that if
+you wish to set this variable, you must do so in the early init
+file.  Regardless of the value of this variable, packages are not
+made available if `user-init-file' is nil (e.g. Emacs was started
+with \"-q\").
 
 Even if the value is nil, you can type \\[package-initialize] to
-activate the package system at any time."
+make installed packages available at any time, or you can
+call (package-initialize) in your init-file."
   :type 'boolean
   :version "24.1")
 
 (defcustom package-load-list '(all)
-  "List of packages for `package-initialize' to load.
+  "List of packages for `package-initialize' to make available.
 Each element in this list should be a list (NAME VERSION), or the
-symbol `all'.  The symbol `all' says to load the latest installed
-versions of all packages not specified by other elements.
+symbol `all'.  The symbol `all' says to make available the latest
+installed versions of all packages not specified by other
+elements.
 
 For an element (NAME VERSION), NAME is a package name (a symbol).
 VERSION should be t, a string, or nil.
-If VERSION is t, the most recent version is activated.
-If VERSION is a string, only that version is ever loaded.
+If VERSION is t, the most recent version is made available.
+If VERSION is a string, only that version is ever made available.
  Any other version, even if newer, is silently ignored.
  Hence, the package is \"held\" at that version.
-If VERSION is nil, the package is not loaded (it is \"disabled\")."
+If VERSION is nil, the package is not made available (it is \"disabled\")."
   :type '(repeat (choice (const all)
                          (list :tag "Specific package"
                                (symbol :tag "Package name")
-- 
2.16.2


[-- Attachment #4: 0001-Warn-if-package-initialize-is-called-twice.patch --]
[-- Type: application/octet-stream, Size: 1674 bytes --]

From 678354ea2159aa0fdf39a5219d112eaebcad4f6e Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 21:06:53 -0800
Subject: [PATCH] Warn if `package-initialize' is called twice

* lisp/emacs-lisp/package.el (package-warn-on-reinitialization): New
variable.
(package-initialize): Conditionally signal a warning.

See: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00626.html
---
 lisp/emacs-lisp/package.el | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 6c307cc37c..7bc369c95d 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -175,6 +175,13 @@ package-enable-at-startup
   :type 'boolean
   :version "24.1")
 
+(defcustom package-warn-on-reinitialization t
+  "Whether to warn when `package-initialize' is called twice.
+If non-nil, and `package-initialize' is called more than once
+before Emacs startup has completed, signal a warning."
+  :type 'boolean
+  :version "27.1")
+
 (defcustom package-load-list '(all)
   "List of packages for `package-initialize' to make available.
 Each element in this list should be a list (NAME VERSION), or the
@@ -1448,6 +1455,10 @@ package-initialize
 individual packages after calling `package-initialize' -- this is
 taken care of by `package-initialize'."
   (interactive)
+  (when (and package--initialized
+             (not after-init-time)
+             package-warn-on-reinitialization)
+    (warn "Unnecessary call to `package-initialize' in init file"))
   (setq package-alist nil)
   (setq package-enable-at-startup nil)
   (package-load-all-descriptors)
-- 
2.16.2

Re: [PATCH] Various fixes for early init file changes

[Clément Pit-Claudel]

On 2018-03-03 00:31, Radon Rosborough wrote:
>  This file is loaded before the package system is initialized, so in it
> -you can customize variables that affect the initialization process,
> -such as @code{package-enable-at-startup} and @code{package-load-list}.
> -@xref{Package Installation}.
> +you can customize variables that affect the initialization process
> +(i.e. @code{package-initialize}), such as

This parenthesis confused me a bit at first (I though you were giving an example of variables that affect the initialization process).  I suggest removing the parenthesis and saying "variables that affect the *package* initialization process".

> +before Emacs startup has completed, signal a warning."

Maybe just "warn"?

Thanks again for the hard work,
Clément.

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

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

> This parenthesis confused me a bit at first
>
> […]
>
> Maybe just "warn"?

OK, attached.

[-- Attachment #2: 0001-Documentation-fixes-for-early-init-file-changes.patch --]
[-- Type: application/octet-stream, Size: 4621 bytes --]

From d29c0d42697a56ad1cd58a0cc8feacee24017ec5 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 20:33:34 -0800
Subject: [PATCH] Documentation fixes for early init file changes

* doc/emacs/custom.texi (Early Init File): Add more details about
which variables must be set in the early init file rather than the
regular init file.  See [1].

* lisp/emacs-lisp/package.el (package-enable-at-startup): Update
docstring to note that packages are now made available before loading
the init file, rather than afterwards.  See [2].
(package-load-list): Refer to "making available" rather than "loading"
for packages.  See [3].

[1]: https://lists.nongnu.org/archive/html/bug-gnu-emacs/2018-02/msg00827.html
[2]: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00632.html
[3]: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00298.html
---
 doc/emacs/custom.texi      | 10 +++++++---
 lisp/emacs-lisp/package.el | 27 ++++++++++++++++-----------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 0905ae7bb1..2ec922ea6a 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -2606,9 +2606,13 @@ Early Init File
 earlier than the normal init file is processed.  Such customizations
 can be put in the early init file, @file{~/.emacs.d/early-init.el}.
 This file is loaded before the package system is initialized, so in it
-you can customize variables that affect the initialization process,
-such as @code{package-enable-at-startup} and @code{package-load-list}.
-@xref{Package Installation}.
+you can customize variables that affect the package initialization
+process, such as @code{package-enable-at-startup},
+@code{package-load-list}, and @code{package-user-dir}.  Note that
+variables like @code{package-archives} which only affect the
+installation of new packages, and not the process of making
+already-installed packages available, may be customized in the regular
+init file.  @xref{Package Installation}.
 
   For more information on the early init file, @pxref{Init File,,,
 elisp, The Emacs Lisp Reference Manual}.
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 999e0d0752..6c307cc37c 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -161,29 +161,34 @@ package
 ;;; Customization options
 ;;;###autoload
 (defcustom package-enable-at-startup t
-  "Whether to activate installed packages when Emacs starts.
-If non-nil, packages are activated after reading the init file
-and before `after-init-hook'.  Activation is not done if
-`user-init-file' is nil (e.g. Emacs was started with \"-q\").
+  "Whether to make installed packages available when Emacs starts.
+If non-nil, packages are made available before reading the init
+file (but after reading the early init file).  This means that if
+you wish to set this variable, you must do so in the early init
+file.  Regardless of the value of this variable, packages are not
+made available if `user-init-file' is nil (e.g. Emacs was started
+with \"-q\").
 
 Even if the value is nil, you can type \\[package-initialize] to
-activate the package system at any time."
+make installed packages available at any time, or you can
+call (package-initialize) in your init-file."
   :type 'boolean
   :version "24.1")
 
 (defcustom package-load-list '(all)
-  "List of packages for `package-initialize' to load.
+  "List of packages for `package-initialize' to make available.
 Each element in this list should be a list (NAME VERSION), or the
-symbol `all'.  The symbol `all' says to load the latest installed
-versions of all packages not specified by other elements.
+symbol `all'.  The symbol `all' says to make available the latest
+installed versions of all packages not specified by other
+elements.
 
 For an element (NAME VERSION), NAME is a package name (a symbol).
 VERSION should be t, a string, or nil.
-If VERSION is t, the most recent version is activated.
-If VERSION is a string, only that version is ever loaded.
+If VERSION is t, the most recent version is made available.
+If VERSION is a string, only that version is ever made available.
  Any other version, even if newer, is silently ignored.
  Hence, the package is \"held\" at that version.
-If VERSION is nil, the package is not loaded (it is \"disabled\")."
+If VERSION is nil, the package is not made available (it is \"disabled\")."
   :type '(repeat (choice (const all)
                          (list :tag "Specific package"
                                (symbol :tag "Package name")
-- 
2.16.2


[-- Attachment #3: 0001-Fix-Info-default-directory-list-error-during-init.patch --]
[-- Type: application/octet-stream, Size: 2197 bytes --]

From b61d09e08302c7441e1f2301287b5fcff4303ecd Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 20:55:48 -0800
Subject: [PATCH] Fix Info-default-directory-list error during init

* lisp/startup.el (command-line): Call `custom-reevaluate-setting' on
predefined variables before loading the early init file and before
`package-initialize' is called.  This prevents
`Info-default-directory-list' from being unbound when
`package-initialize' tries to access it during startup.  See [1].

[1]: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00545.html
---
 lisp/startup.el | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index 4105c1db2d..2669342eda 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -1115,6 +1115,15 @@ command-line
     (and command-line-args
          (setcdr command-line-args args)))
 
+  ;; Re-evaluate predefined variables whose initial value depends on
+  ;; the runtime context.
+  (let (current-load-list) ; c-r-s may call defvar, and hence LOADHIST_ATTACH
+    (mapc 'custom-reevaluate-setting
+          ;; Initialize them in the same order they were loaded, in case there
+          ;; are dependencies between them.
+          (prog1 (nreverse custom-delayed-init-variables)
+            (setq custom-delayed-init-variables nil))))
+
   ;; Warn for invalid user name.
   (when init-file-user
     (if (string-match "[~/:\n]" init-file-user)
@@ -1245,15 +1254,6 @@ command-line
     (startup--setup-quote-display)
     (setq internal--text-quoting-flag t))
 
-  ;; Re-evaluate predefined variables whose initial value depends on
-  ;; the runtime context.
-  (let (current-load-list) ; c-r-s may call defvar, and hence LOADHIST_ATTACH
-    (mapc 'custom-reevaluate-setting
-          ;; Initialize them in the same order they were loaded, in case there
-          ;; are dependencies between them.
-          (prog1 (nreverse custom-delayed-init-variables)
-            (setq custom-delayed-init-variables nil))))
-
   (normal-erase-is-backspace-setup-frame)
 
   ;; Register default TTY colors for the case the terminal hasn't a
-- 
2.16.2


[-- Attachment #4: 0001-Warn-if-package-initialize-is-called-twice.patch --]
[-- Type: application/octet-stream, Size: 1659 bytes --]

From fa6a7badc64edd2e2c098b538053a622b11e91d4 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 21:06:53 -0800
Subject: [PATCH] Warn if `package-initialize' is called twice

* lisp/emacs-lisp/package.el (package-warn-on-reinitialization): New
variable.
(package-initialize): Conditionally signal a warning.

See: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00626.html
---
 lisp/emacs-lisp/package.el | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 6c307cc37c..d821596216 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -175,6 +175,13 @@ package-enable-at-startup
   :type 'boolean
   :version "24.1")
 
+(defcustom package-warn-on-reinitialization t
+  "Whether to warn when `package-initialize' is called twice.
+This only warns if `package-initialize' is called more than once
+before Emacs startup has completed."
+  :type 'boolean
+  :version "27.1")
+
 (defcustom package-load-list '(all)
   "List of packages for `package-initialize' to make available.
 Each element in this list should be a list (NAME VERSION), or the
@@ -1448,6 +1455,10 @@ package-initialize
 individual packages after calling `package-initialize' -- this is
 taken care of by `package-initialize'."
   (interactive)
+  (when (and package--initialized
+             (not after-init-time)
+             package-warn-on-reinitialization)
+    (warn "Unnecessary call to `package-initialize' in init file"))
   (setq package-alist nil)
   (setq package-enable-at-startup nil)
   (package-load-all-descriptors)
-- 
2.16.2

Re: [PATCH] Various fixes for early init file changes

[Noam Postavsky]

On Sat, Mar 3, 2018 at 12:31 AM, Radon Rosborough <radon.neon@gmail.com> wrote:

> 3. Warn if `package-initialize' is called twice. This causes Emacs to
> emit a warning, by default, if `package-initialize' is called twice
> during init. I expect this to cause people a lot of warnings, but I
> think this is a good thing, since the alternative (currently) is a
> pretty bad performance regression if people leave a now-superfluous
> extra call to `package-initialize' in their init files. Plus, the fix
> is trivial: just delete the extra call. For people who really do want
> to call `package-initialize' twice for some reason, there is a new
> variable `package-warn-on-reinitialization' which can be customized to
> disable the warning.

You could use

(lwarn '(package reinitialize) :warning
       "Unnecessary call to `package-initialize' in init file")

and then the warning could be suppressed by adding '(package
reinitialize) to warning-suppress-types instead of creating a new
custom option.

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

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

> the warning could be suppressed by adding '(package
> reinitialize) to warning-suppress-types instead of creating a new
> custom option.

I just typed up a whole email explaining why it would be better for
novice users if the process for this was not so complicated.

Then I realized that novice users should not be suppressing this
warning, and should instead be taking the extra call to
`package-initialize' out of their init-files.

As a result, I now wholeheartedly agree with your idea, and I've
attached a revised patch. Thanks for the feedback.

[-- Attachment #2: 0001-Warn-if-package-initialize-is-called-twice.patch --]
[-- Type: application/octet-stream, Size: 1770 bytes --]

From 53067f7b3d471ff0e993293bebe5f865ca69b43a Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Fri, 2 Mar 2018 21:06:53 -0800
Subject: [PATCH] Warn if `package-initialize' is called twice

* lisp/emacs-lisp/package.el (package-initialize): Conditionally
signal a warning.

See: https://lists.gnu.org/archive/html/emacs-devel/2018-02/msg00626.html
     https://lists.gnu.org/archive/html/emacs-devel/2018-03/msg00301.html
---
 lisp/emacs-lisp/package.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 6c307cc37c..1edc06d024 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1444,10 +1444,21 @@ package-initialize
 If called as part of loading `user-init-file', set
 `package-enable-at-startup' to nil, to prevent accidentally
 loading packages twice.
+
 It is not necessary to adjust `load-path' or `require' the
 individual packages after calling `package-initialize' -- this is
-taken care of by `package-initialize'."
+taken care of by `package-initialize'.
+
+If `package-initialize' is called twice during Emacs startup,
+signal a warning, since this is a bad idea except in highly
+advanced use cases.  To suppress the warning, remove the
+superfluous call to `package-initialize' from your init-file.  If
+you have code which must run before `package-initialize', put
+that code in the early init-file."
   (interactive)
+  (when (and package--initialized (not after-init-time))
+    (lwarn '(package reinitialization) :warning
+           "Unnecessary call to `package-initialize' in init file"))
   (setq package-alist nil)
   (setq package-enable-at-startup nil)
   (package-load-all-descriptors)
-- 
2.16.2

Re: [PATCH] Various fixes for early init file changes

[Noam Postavsky]

On Fri, Mar 9, 2018 at 1:07 AM, Radon Rosborough <radon.neon@gmail.com> wrote:
>> the warning could be suppressed by adding '(package
>> reinitialize) to warning-suppress-types instead of creating a new
>> custom option.
>
> I just typed up a whole email explaining why it would be better for
> novice users if the process for this was not so complicated.

This reminded me that the *Warnings* buffer could use some
enhancements (this is not directly related to your patches). I opened
Bug#30757 for it.

https://debbugs.gnu.org/30757

Re: [PATCH] Various fixes for early init file changes

[Eli Zaretskii]

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Thu, 8 Mar 2018 22:07:05 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > the warning could be suppressed by adding '(package
> > reinitialize) to warning-suppress-types instead of creating a new
> > custom option.
> 
> I just typed up a whole email explaining why it would be better for
> novice users if the process for this was not so complicated.
> 
> Then I realized that novice users should not be suppressing this
> warning, and should instead be taking the extra call to
> `package-initialize' out of their init-files.
> 
> As a result, I now wholeheartedly agree with your idea, and I've
> attached a revised patch. Thanks for the feedback.

Thanks, I pushed this and other 2 patches to the master branch.

Re: [PATCH] Various fixes for early init file changes

[Robert Pluim]

Radon Rosborough <radon.neon@gmail.com> writes:

>> the warning could be suppressed by adding '(package
>> reinitialize) to warning-suppress-types instead of creating a new
>> custom option.
>
> I just typed up a whole email explaining why it would be better for
> novice users if the process for this was not so complicated.
>
> Then I realized that novice users should not be suppressing this
> warning, and should instead be taking the extra call to
> `package-initialize' out of their init-files.

Hmm, in light of this, should we abandon the idea of adding buttons in
*Warning* buffers allowing people to easily suppress warnings? Or are
there other use cases?

Robert

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

>> Then I realized that novice users should not be suppressing this
>> warning, and should instead be taking the extra call to
>> `package-initialize' out of their init-files.
>
> Hmm, in light of this, should we abandon the idea of adding buttons in
> *Warning* buffers allowing people to easily suppress warnings? Or are
> there other use cases?

I'm honestly not sure. I like the idea of making *Warnings* more
user-friendly, because that seems like a good step towards
modernization. However, I do think that there shouldn't be any easily
accessible way to turn off this one particular warning, since
otherwise I can guarantee a lot of people are going to use it instead
of fixing the (important) problem.

Maybe as a compromise there could be a way to mark a warning so that a
disable button isn't shown for it?

-- Radon

Re: [PATCH] Various fixes for early init file changes

[Stefan Monnier]

> Maybe as a compromise there could be a way to mark a warning so that a
> disable button isn't shown for it?

Better would be to replace this "disable warning" button with a "go fix
it" button which tries to jump to the source of the problem (e.g. opens
the .emacs file, looks for package-initialize, and emits a message
saying that this call should likely be removed (or highlights the
package-initialize call with such a message)).


        Stefan

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

>> Maybe as a compromise there could be a way to mark a warning so that a
>> disable button isn't shown for it?
>
> Better would be to replace this "disable warning" button with a "go fix
> it" button which tries to jump to the source of the problem (e.g. opens
> the .emacs file, looks for package-initialize, and emits a message
> saying that this call should likely be removed (or highlights the
> package-initialize call with such a message)).

Great idea, I would love to see that.

Re: [PATCH] Various fixes for early init file changes

[Stefan Monnier]

>> Better would be to replace this "disable warning" button with a "go fix
>> it" button which tries to jump to the source of the problem (e.g. opens
>> the .emacs file, looks for package-initialize, and emits a message
>> saying that this call should likely be removed (or highlights the
>> package-initialize call with such a message)).
>
> Great idea, I would love to see that.

Radon told me he's already working on a patch doing just that


        Stefan ;-)

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

>> Great idea, I would love to see that.
>
> Radon told me he's already working on a patch doing just that

What?

RE: [PATCH] Various fixes for early init file changes

[Drew Adams]

> >> Then I realized that novice users should not be suppressing this
> >> warning, and should instead be taking the extra call to
> >> `package-initialize' out of their init-files.
> >
> > Hmm, in light of this, should we abandon the idea of adding buttons in
> > *Warning* buffers allowing people to easily suppress warnings? Or are
> > there other use cases?
> 
> I'm honestly not sure. I like the idea of making *Warnings* more
> user-friendly, because that seems like a good step towards
> modernization. However, I do think that there shouldn't be any easily
> accessible way to turn off this one particular warning, since
> otherwise I can guarantee a lot of people are going to use it instead
> of fixing the (important) problem.
> 
> Maybe as a compromise there could be a way to mark a warning so that a
> disable button isn't shown for it?

IMO:

Disabling should be easy for anyone, including novices.

However, clicking a disable-warning button can display an
explanation of the warning and say why you might NOT want
to disable it.  Confirmation can then be required.

IOW, (1) make it easy, but (2) require confirmation and
(3) provide explanation of what's involved.

The idea is to teach, not inhibit, and then let users decide.

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

> Disabling should be easy for anyone, including novices.

I agree with your philosophy but in this case allowing such disabling
would actually do more harm than good. It's not a matter of a choice
being made for the user, it's just a case of their init-file is wrong
and needs to be fixed (otherwise it will load slower). Let's be
pragmatic here and recognize that since basically every user of Emacs
is going to see this message, if there is any way for the user to mess
up, it will happen hundreds of times.

Stefan's suggestion is of course the right answer, I think: the user
is given an option to fix the warning, not disable it.

Re: [PATCH] Various fixes for early init file changes

[Clément Pit-Claudel]

On 2018-03-19 16:16, Radon Rosborough wrote:
>> Disabling should be easy for anyone, including novices.
> 
> I agree with your philosophy but in this case allowing such disabling
> would actually do more harm than good. It's not a matter of a choice
> being made for the user, it's just a case of their init-file is wrong
> and needs to be fixed (otherwise it will load slower). Let's be
> pragmatic here and recognize that since basically every user of Emacs
> is going to see this message, if there is any way for the user to mess
> up, it will happen hundreds of times.
> 
> Stefan's suggestion is of course the right answer, I think: the user
> is given an option to fix the warning, not disable it.

There is another change we could consider: automatically removing the automatically-added call to package-initialize.

Re: [PATCH] Various fixes for early init file changes

[Radon Rosborough]

> There is another change we could consider: automatically removing the automatically-added call to package-initialize.

We want to be careful to avoid unnecessary tampering with the user's
init-file, since my annoyance with that was the whole reason I pushed
this change through. That said, I think it would be reasonable to
automagically remove it as long as it's accompanied by the "Added by
Package.el" comment which identifies it as definitely not
user-written.

Re: [PATCH] Various fixes for early init file changes

[Clément Pit-Claudel]

On 2018-03-20 01:44, Radon Rosborough wrote:
>> There is another change we could consider: automatically removing
>> the automatically-added call to package-initialize.
> 
> […] I think it would be reasonable to automagically remove it
> as long as it's accompanied by the "Added by Package.el" comment
> which identifies it as definitely not user-written.

Exactly :)

Re: Various fixes for early init file changes

[Andy Moreton]

On Mon 19 Mar 2018, Radon Rosborough wrote:

>> There is another change we could consider: automatically removing the automatically-added call to package-initialize.
>
> We want to be careful to avoid unnecessary tampering with the user's
> init-file, since my annoyance with that was the whole reason I pushed
> this change through. That said, I think it would be reasonable to
> automagically remove it as long as it's accompanied by the "Added by
> Package.el" comment which identifies it as definitely not
> user-written.

It was a mistake to automatically add code to the user's init file.
Automatically deleting code from the user's init file is even worse - I
cannot imagine any circumstance in which automatic deletion is
acceptable.

    AndyM

Re: Various fixes for early init file changes

[Radon Rosborough]

> I cannot imagine any circumstance in which automatic deletion is
> acceptable.

Well, I am sympathetic to your viewpoint. However, I would qualify
your statement and say that any automatic modification would be bad if
there is at least one person whom it would hurt.

Certainly the original insertion of (package-initialize) hurt lots of
people, for example people who didn't want to use package.el. However,
reversing this change has the positive impact of speeding up people's
init times by as much as a factor of two in some cases, and it would
*only* affect people who had the comment automatically added (so they
didn't add it intentionally as part of their configuration).

Can anyone think of a case in which this automatic deletion might
cause a problem or be annoying?

Re: Various fixes for early init file changes

[Robert Pluim]

Radon Rosborough <radon.neon@gmail.com> writes:

> Can anyone think of a case in which this automatic deletion might
> cause a problem or be annoying?

Yes. I switch between emacs-26 and emacs-master a fair amount. Having
that affect my .emacs would be very annoying (I keep .emacs under
version control).

Iʼll admit itʼs not a typical use case, though.

Robert

Re: Various fixes for early init file changes

[Radon Rosborough]

> I switch between emacs-26 and emacs-master a fair amount. Having
> that affect my .emacs would be very annoying (I keep .emacs under
> version control).

But if your init-file contains (package-initialize) without the
"explanatory comments", it would be modified in neither Emacs 26 nor
Emacs 27.

Re: Various fixes for early init file changes

[Robert Pluim]

Radon Rosborough <radon.neon@gmail.com> writes:

>> I switch between emacs-26 and emacs-master a fair amount. Having
>> that affect my .emacs would be very annoying (I keep .emacs under
>> version control).
>
> But if your init-file contains (package-initialize) without the
> "explanatory comments", it would be modified in neither Emacs 26 nor
> Emacs 27.

Ah, Iʼd missed the 'explanatory comments' bit. OK for me then (but Iʼm
sure others will object to Emacs messing with their init file).

Robert