unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Fix `early-init-file' value when file is missing
@ 2019-01-16  5:54 Radon Rosborough
  2019-01-23  3:47 ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-01-16  5:54 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 468 bytes --]

Dear all,

In Emacs 27, when Emacs starts up without an early init-file (located
by default at ~/.emacs.d/early-init.el), the value of
`early-init-file' is set to ~/.emacs.d/early-init (note the missing
file extension). When the early init-file does exist, the variable is
set to the correct value. I have attached a patch which fixes this
bug.

Feedback is welcome. Please copy me on replies, since I am not
subscribed to emacs-devel.

Best regards,
Radon Rosborough

[-- Attachment #1.2: Type: text/html, Size: 620 bytes --]

[-- Attachment #2: 0001-Fix-early-init-file-value-when-file-is-missing.patch --]
[-- Type: application/octet-stream, Size: 1964 bytes --]

From cca8239869acf0de37cd5ae0b81a6a103be354f0 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Sun, 13 Jan 2019 21:55:42 -0700
Subject: [PATCH] Fix `early-init-file' value when file is missing

Previously, if no early init-file existed in `user-emacs-directory',
then the value of `early-init-file' after startup would be
~/.emacs.d/early-init (note the missing extension).  This commit
adjusts that value to ~/.emacs.d/early-init.el as desired, while not
changing other behavior.  Note that when the early init-file did
exist, then the value of `early-init-file' after startup was already
correct; this commit fixes a bug that occurred only when the file did
not exist.

lisp/startup.el (load-user-init-file): Update logic.
---
 lisp/startup.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index 1011d5f953..cd1e296686 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -923,7 +923,17 @@ load-user-init-file
               ;; user-init-file conclusively.  Don't let it be
               ;; set from default.el.
               (when (eq user-init-file t)
-                (setq user-init-file init-file-name)))
+                (setq user-init-file
+                      ;; If the init-file doesn't exist, default to
+                      ;; the .el version, not the bare filename.
+                      ;; (Practically, this means that
+                      ;; `early-init-file' will be set to the value
+                      ;; "early-init.el" rather than the more
+                      ;; confusing value "early-init" during startup
+                      ;; if the file does not exist.)
+                      (concat
+                       (file-name-sans-extension init-file-name)
+                       ".el"))))
 
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-01-16  5:54 [PATCH] Fix `early-init-file' value when file is missing Radon Rosborough
@ 2019-01-23  3:47 ` Radon Rosborough
  2019-01-29 17:24   ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-01-23  3:47 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 596 bytes --]

> In Emacs 27, when Emacs starts up without an early init-file (located
> by default at ~/.emacs.d/early-init.el), the value of
> `early-init-file' is set to ~/.emacs.d/early-init (note the missing
> file extension). When the early init-file does exist, the variable is
> set to the correct value. I have attached a patch which fixes this
> bug.
>
> Feedback is welcome. Please copy me on replies, since I am not
> subscribed to emacs-devel.

I am following up on this message since it has been one week with no
reply, and I would like to see the patch either merged or discussed.

Thanks,
Radon

[-- Attachment #1.2: Type: text/html, Size: 791 bytes --]

[-- Attachment #2: 0001-Fix-early-init-file-value-when-file-is-missing.patch --]
[-- Type: application/octet-stream, Size: 1964 bytes --]

From cca8239869acf0de37cd5ae0b81a6a103be354f0 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Sun, 13 Jan 2019 21:55:42 -0700
Subject: [PATCH] Fix `early-init-file' value when file is missing

Previously, if no early init-file existed in `user-emacs-directory',
then the value of `early-init-file' after startup would be
~/.emacs.d/early-init (note the missing extension).  This commit
adjusts that value to ~/.emacs.d/early-init.el as desired, while not
changing other behavior.  Note that when the early init-file did
exist, then the value of `early-init-file' after startup was already
correct; this commit fixes a bug that occurred only when the file did
not exist.

lisp/startup.el (load-user-init-file): Update logic.
---
 lisp/startup.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index 1011d5f953..cd1e296686 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -923,7 +923,17 @@ load-user-init-file
               ;; user-init-file conclusively.  Don't let it be
               ;; set from default.el.
               (when (eq user-init-file t)
-                (setq user-init-file init-file-name)))
+                (setq user-init-file
+                      ;; If the init-file doesn't exist, default to
+                      ;; the .el version, not the bare filename.
+                      ;; (Practically, this means that
+                      ;; `early-init-file' will be set to the value
+                      ;; "early-init.el" rather than the more
+                      ;; confusing value "early-init" during startup
+                      ;; if the file does not exist.)
+                      (concat
+                       (file-name-sans-extension init-file-name)
+                       ".el"))))
 
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-01-23  3:47 ` Radon Rosborough
@ 2019-01-29 17:24   ` Radon Rosborough
  2019-01-29 17:46     ` Eli Zaretskii
  2019-02-01  9:11     ` Eli Zaretskii
  0 siblings, 2 replies; 19+ messages in thread
From: Radon Rosborough @ 2019-01-29 17:24 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 602 bytes --]

> In Emacs 27, when Emacs starts up without an early init-file (located
> by default at ~/.emacs.d/early-init.el), the value of
> `early-init-file' is set to ~/.emacs.d/early-init (note the missing
> file extension). When the early init-file does exist, the variable is
> set to the correct value. I have attached a patch which fixes this
> bug.
>
> Feedback is welcome. Please copy me on replies, since I am not
> subscribed to emacs-devel.

Hi. I am following up on this again since it has been two weeks with
no reply. Could somebody please let me know that the message was
received?

Thanks,
Radon

[-- Attachment #1.2: Type: text/html, Size: 810 bytes --]

[-- Attachment #2: 0001-Fix-early-init-file-value-when-file-is-missing.patch --]
[-- Type: application/octet-stream, Size: 1964 bytes --]

From cca8239869acf0de37cd5ae0b81a6a103be354f0 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Sun, 13 Jan 2019 21:55:42 -0700
Subject: [PATCH] Fix `early-init-file' value when file is missing

Previously, if no early init-file existed in `user-emacs-directory',
then the value of `early-init-file' after startup would be
~/.emacs.d/early-init (note the missing extension).  This commit
adjusts that value to ~/.emacs.d/early-init.el as desired, while not
changing other behavior.  Note that when the early init-file did
exist, then the value of `early-init-file' after startup was already
correct; this commit fixes a bug that occurred only when the file did
not exist.

lisp/startup.el (load-user-init-file): Update logic.
---
 lisp/startup.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index 1011d5f953..cd1e296686 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -923,7 +923,17 @@ load-user-init-file
               ;; user-init-file conclusively.  Don't let it be
               ;; set from default.el.
               (when (eq user-init-file t)
-                (setq user-init-file init-file-name)))
+                (setq user-init-file
+                      ;; If the init-file doesn't exist, default to
+                      ;; the .el version, not the bare filename.
+                      ;; (Practically, this means that
+                      ;; `early-init-file' will be set to the value
+                      ;; "early-init.el" rather than the more
+                      ;; confusing value "early-init" during startup
+                      ;; if the file does not exist.)
+                      (concat
+                       (file-name-sans-extension init-file-name)
+                       ".el"))))
 
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-01-29 17:24   ` Radon Rosborough
@ 2019-01-29 17:46     ` Eli Zaretskii
  2019-02-01  9:11     ` Eli Zaretskii
  1 sibling, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2019-01-29 17:46 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Tue, 29 Jan 2019 09:24:07 -0800
> 
> Could somebody please let me know that the message was received?

The message was received.

Thanks.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-01-29 17:24   ` Radon Rosborough
  2019-01-29 17:46     ` Eli Zaretskii
@ 2019-02-01  9:11     ` Eli Zaretskii
  2019-02-01 23:10       ` Radon Rosborough
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-01  9:11 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Tue, 29 Jan 2019 09:24:07 -0800
> 
> Hi. I am following up on this again since it has been two weeks with
> no reply. Could somebody please let me know that the message was
> received?

Sorry for the long delay.

This patch has the (possibly unintended) consequence that if ~/.emacs
doesn't exist, user-init-file is set to "~/.emacs.el", something I
don't think we want.  Can you propose a change that will affect only
early-init-file, not any other init file?

Thanks.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-01  9:11     ` Eli Zaretskii
@ 2019-02-01 23:10       ` Radon Rosborough
  2019-02-08  7:32         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-02-01 23:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 496 bytes --]

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Feb 1, 2019, 1:11 AM
>
> This patch has the (possibly unintended) consequence that if ~/.emacs
> doesn't exist, user-init-file is set to "~/.emacs.el", something I
> don't think we want. Can you propose a change that will affect only
> early-init-file, not any other init file?

Sure, no problem. I apologize for the oversight.

A revised patch which implements the same bugfix without changing the
value of `user-init-file' is attached.

Best,
Radon

[-- Attachment #1.2: Type: text/html, Size: 736 bytes --]

[-- Attachment #2: 0001-Fix-early-init-file-value-when-file-is-missing.patch --]
[-- Type: application/octet-stream, Size: 5471 bytes --]

From 729fb4b7dc1ec6512db8838c6a4fff8d5810e115 Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Sun, 13 Jan 2019 21:55:42 -0700
Subject: [PATCH] Fix `early-init-file' value when file is missing

Previously, if no early init-file existed in `user-emacs-directory',
then the value of `early-init-file' after startup would be
~/.emacs.d/early-init (note the missing extension).  This commit
adjusts that value to ~/.emacs.d/early-init.el as desired, while not
changing other behavior.  Note that when the early init-file did
exist, then the value of `early-init-file' after startup was already
correct; this commit fixes a bug that occurred only when the file did
not exist.

lisp/startup.el (load-user-init-file): Update logic.
---
 lisp/startup.el | 57 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 18 deletions(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index f2410f6f2c..367b04ad71 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -887,17 +887,22 @@ startup--setup-quote-display
             (aset standard-display-table char nil)))))))
 
 (defun load-user-init-file
-    (filename-function &optional alternate-filename-function load-defaults)
+    (filename-function &optional old-filename-function load-defaults)
   "Load a user init-file.
 FILENAME-FUNCTION is called with no arguments and should return
-the name of the init-file to load.  If this file cannot be
-loaded, and ALTERNATE-FILENAME-FUNCTION is non-nil, then it is
-called with no arguments and should return the name of an
-alternate init-file to load.  If LOAD-DEFAULTS is non-nil, then
-load default.el after the init-file.
-
-This function sets `user-init-file' to the name of the loaded
-init-file, or to a default value if loading is not possible."
+the name of the init-file to load.  If OLD-FILENAME-FUNCTION is
+non-nil, then try it first, and use FILENAME-FUNCTION only if the
+file cannot be found.
+
+Set `user-init-file' to the name of the loaded init-file, or to a
+default value if loading is not possible.  This default value
+will end in .el, unless OLD-FILENAME-FUNCTION is nil (this caveat
+being for backwards compatibility).  Additionally, if the
+filename of the loaded init-file ended in .elc, then replace it
+with .el, unconditionally.
+
+If LOAD-DEFAULTS is non-nil, then load default.el after the
+init-file."
   (let ((debug-on-error-from-init-file nil)
         (debug-on-error-should-be-set nil)
         (debug-on-error-initial
@@ -907,7 +912,9 @@ load-user-init-file
     (let ((debug-on-error debug-on-error-initial))
       (condition-case-unless-debug error
           (when init-file-user
-            (let ((init-file-name (funcall filename-function)))
+            ;; The first filename to try is the old one, if provided.
+            (let ((init-file-name
+                   (funcall (or old-filename-function filename-function))))
 
               ;; If `user-init-file' is t, then `load' will store
               ;; the name of the file that it loads into
@@ -915,15 +922,29 @@ load-user-init-file
               (setq user-init-file t)
               (load init-file-name 'noerror 'nomessage)
 
-              (when (and (eq user-init-file t) alternate-filename-function)
-                (load (funcall alternate-filename-function)
+              ;; If we were given an old filename and couldn't load
+              ;; it, then try the new filename.
+              (when (and (eq user-init-file t) old-filename-function)
+                (load (funcall filename-function)
                       'noerror 'nomessage))
 
               ;; If we did not find the user's init file, set
               ;; user-init-file conclusively.  Don't let it be
               ;; set from default.el.
               (when (eq user-init-file t)
-                (setq user-init-file init-file-name)))
+                (setq user-init-file
+                      ;; If the init-file doesn't exist, default to
+                      ;; the .el version, not the bare filename.  But
+                      ;; for backwards compatibility, don't do this if
+                      ;; an OLD-FILENAME-FUNCTION was provided (so we
+                      ;; use ~/.emacs instead of ~/.emacs.el, but
+                      ;; ~/.emacs.d/early-init.el instead of
+                      ;; ~/.emacs.d/early-init, by default).
+                      (if old-filename-function
+                          init-file-name
+                        (concat
+                         (file-name-sans-extension init-file-name)
+                         ".el")))))
 
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
@@ -1305,6 +1326,11 @@ command-line
 
     ;; Load that user's init file, or the default one, or none.
     (load-user-init-file
+     (lambda ()
+       (expand-file-name
+        "init"
+        (file-name-as-directory
+         (concat "~" init-file-user "/.emacs.d"))))
      (lambda ()
        (cond
         ((eq system-type 'ms-dos)
@@ -1324,11 +1350,6 @@ command-line
          "~/_emacs")
         (t ;; But default to .emacs if _emacs does not exist.
          "~/.emacs")))
-     (lambda ()
-       (expand-file-name
-        "init"
-        (file-name-as-directory
-         (concat "~" init-file-user "/.emacs.d"))))
      (not inhibit-default-init))
 
     (when (and deactivate-mark transient-mark-mode)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-01 23:10       ` Radon Rosborough
@ 2019-02-08  7:32         ` Eli Zaretskii
  2019-02-08 17:34           ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-08  7:32 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Fri, 1 Feb 2019 15:10:24 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > This patch has the (possibly unintended) consequence that if ~/.emacs
> > doesn't exist, user-init-file is set to "~/.emacs.el", something I
> > don't think we want. Can you propose a change that will affect only
> > early-init-file, not any other init file?
> 
> Sure, no problem. I apologize for the oversight.
> 
> A revised patch which implements the same bugfix without changing the
> value of `user-init-file' is attached.
> 
> Previously, if no early init-file existed in `user-emacs-directory',
> then the value of `early-init-file' after startup would be
> ~/.emacs.d/early-init (note the missing extension).  This commit
> adjusts that value to ~/.emacs.d/early-init.el as desired, while not
> changing other behavior.  Note that when the early init-file did
> exist, then the value of `early-init-file' after startup was already
> correct; this commit fixes a bug that occurred only when the file did
> not exist.
> 
> lisp/startup.el (load-user-init-file): Update logic.

Thanks.  However, I would prefer to have this solved outside
load-user-init-file, if feasible.  This function is too central to
Emacs to make non-trivial changes there for such a minor problem's
sake.  Especially since we don't have a test suite for startup
functionalities.  I think the change outside of the function will also
be much simpler and thus easier to grasp.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-08  7:32         ` Eli Zaretskii
@ 2019-02-08 17:34           ` Radon Rosborough
  2019-02-08 21:52             ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-02-08 17:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Feb 7, 2019, 11:32 PM
>
> However, I would prefer to have this solved outside
> load-user-init-file, if feasible. This function is too central to
> Emacs to make non-trivial changes there for such a minor problem's
> sake.

I am sorry, but I simply cannot see how this change is non-trivial.
There is exactly one logic change that this patch makes: namely, that
the extension of user-init-file is changed to .el, but only in the
case of the early init-file not being found. That change will affect
only one of the two invocations of this function which exist, in only
one case. Everything else in the patch is simply renaming variables
and updating the documentation.

I also cannot see how any other way of solving this bug could be as
clear and straightforward as fixing it directly by updating the code
that was broken.

Could you explain your reasoning?

Thanks,
Radon

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-08 17:34           ` Radon Rosborough
@ 2019-02-08 21:52             ` Eli Zaretskii
  2019-02-10 23:04               ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-08 21:52 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Fri, 8 Feb 2019 09:34:08 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > However, I would prefer to have this solved outside
> > load-user-init-file, if feasible. This function is too central to
> > Emacs to make non-trivial changes there for such a minor problem's
> > sake.
> 
> I am sorry, but I simply cannot see how this change is non-trivial.
> There is exactly one logic change that this patch makes: namely, that
> the extension of user-init-file is changed to .el, but only in the
> case of the early init-file not being found.

But to do that, you've modified the API of load-user-init-file,
changing the semantics of its second argument, which then caused the
code of the function to change to accommodate that.  This then
requires to go and check that these changes didn't affect the other
callers of that function in ways we don't want.

> I also cannot see how any other way of solving this bug could be as
> clear and straightforward as fixing it directly by updating the code
> that was broken.

All you want is to set a single variable to a specific value if the
file wasn't found, right?  How hard can it be to do that after
load-user-init-file returns and reports a failure?  Or do that inside
the function, but only if it processes early-init file specifically?

> Could you explain your reasoning?

I tried in the message you responded to, but you've elided all those
explanations.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-08 21:52             ` Eli Zaretskii
@ 2019-02-10 23:04               ` Radon Rosborough
  2019-02-10 23:14                 ` Stefan Monnier
  2019-02-11 16:05                 ` Eli Zaretskii
  0 siblings, 2 replies; 19+ messages in thread
From: Radon Rosborough @ 2019-02-10 23:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> you've modified the API of load-user-init-file

But we have never released a version of Emacs that includes this
function -- and it's an internal function. There are only two callers
of this function that exist, and they are both touched by this patch.

But, okay, the only reason I changed the arguments was because I
thought it was an improvement. Would you accept a patch that fixed
this bug without changing the arguments?

> All you want is to set a single variable to a specific value if the
> file wasn't found, right?

Not quite -- load-user-init-file *already* sets the variable to a
specific value, and the value is wrong. This patch changes the
specific value from the wrong value to the correct value.

> How hard can it be to do that after load-user-init-file returns and
> reports a failure?

I do not know how to do this, because it doesn't seem to me that
load-user-init-file reports failures at all. Instead, it handles them
itself. Hence why changing load-user-init-file is required.

> Or do that inside the function, but only if it processes early-init
> file specifically?

I really thought that this was exactly what my patch did. There are
exactly two places where this internal function is called, and
checking whether the optional argument is provided is the way to tell
whether the early init-file is being processed.

---

In conclusion, if I remove all the changes in this patch except for
the '(setq user-init-file ...)' line, would you accept that?

Best,
Radon

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-10 23:04               ` Radon Rosborough
@ 2019-02-10 23:14                 ` Stefan Monnier
  2019-02-11 16:05                 ` Eli Zaretskii
  1 sibling, 0 replies; 19+ messages in thread
From: Stefan Monnier @ 2019-02-10 23:14 UTC (permalink / raw)
  To: emacs-devel

> But we have never released a version of Emacs that includes this
> function -- and it's an internal function.

Good point: could you put a "--" in its name to clarify this?


        Stefan




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-10 23:04               ` Radon Rosborough
  2019-02-10 23:14                 ` Stefan Monnier
@ 2019-02-11 16:05                 ` Eli Zaretskii
  2019-02-12  5:38                   ` Radon Rosborough
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-11 16:05 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Sun, 10 Feb 2019 15:04:59 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > you've modified the API of load-user-init-file
> 
> But we have never released a version of Emacs that includes this
> function -- and it's an internal function. There are only two callers
> of this function that exist, and they are both touched by this patch.

True, but that's not what bothered me.  What bothered me was that
changes in the interface and the implementation of the function might
adversely affect the other caller of this function in startup.el, and
verifying that no use case will become broken because of that is not a
trivial task.  For a significant improvement ion functionality, such
risks might be justified, but here we are talking about a minor
cleanup, so I'd prefer a safer change.

> > Or do that inside the function, but only if it processes early-init
> > file specifically?
> 
> I really thought that this was exactly what my patch did. There are
> exactly two places where this internal function is called, and
> checking whether the optional argument is provided is the way to tell
> whether the early init-file is being processed.

Such a test is too indirect, and will cease to DTRT if we ever need to
pass a second function in the case of early-init or not pass it in the
other case.  I hope we can make a more direct test.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-11 16:05                 ` Eli Zaretskii
@ 2019-02-12  5:38                   ` Radon Rosborough
  2019-02-12 16:14                     ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-02-12  5:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> I hope we can make a more direct test.

Okay, but as I mentioned in my previous message, I could amend this
patch to not change the signature of `load-user-init-file' (even
though the change has no logical impact whatsoever that could change
user-facing behavior). Would you accept the patch if it didn't change
the signature of `load-user-init-file'?

Thanks,
Radon

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-12  5:38                   ` Radon Rosborough
@ 2019-02-12 16:14                     ` Eli Zaretskii
  2019-02-13  2:36                       ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-12 16:14 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Mon, 11 Feb 2019 21:38:36 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > I hope we can make a more direct test.
> 
> Okay, but as I mentioned in my previous message, I could amend this
> patch to not change the signature of `load-user-init-file' (even
> though the change has no logical impact whatsoever that could change
> user-facing behavior). Would you accept the patch if it didn't change
> the signature of `load-user-init-file'?

I think so, but it's hard to say without actually seeing the patch, or
at least hearing the idea which you intend to implement in a bit more
detail.

Let me just be clear: the most important aspect for me in this regard
is to be able to tell easily that the change affects only the
early-init file and cannot possibly affect the other caller, and also
that the change is obviously correct.  So if you can come up with such
a safe change, I will of course gladly accept it.

Thanks in advance, and sorry for causing you extra work.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-12 16:14                     ` Eli Zaretskii
@ 2019-02-13  2:36                       ` Radon Rosborough
  2019-02-13 17:56                         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-02-13  2:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 100 bytes --]

> if you can come up with such a safe change, I will of course gladly
> accept it.

It is attached.

[-- Attachment #1.2: Type: text/html, Size: 161 bytes --]

[-- Attachment #2: 0001-Fix-early-init-file-value-when-file-is-missing.patch --]
[-- Type: application/octet-stream, Size: 2103 bytes --]

From c6c94fe3d04acd21b52d4a85a334dbe6f13c45ff Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Tue, 12 Feb 2019 18:34:41 -0800
Subject: [PATCH] Fix `early-init-file' value when file is missing

Previously, if no early init-file existed in `user-emacs-directory',
then the value of `early-init-file' after startup would be
~/.emacs.d/early-init (note the missing extension).  This commit
adjusts that value to ~/.emacs.d/early-init.el as desired, while not
changing other behavior.  Note that when the early init-file did
exist, then the value of `early-init-file' after startup was already
correct; this commit fixes a bug that occurred only when the file did
not exist.

lisp/startup.el (load-user-init-file): Update logic.
---
 lisp/startup.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lisp/startup.el b/lisp/startup.el
index f2410f6f2c..eec025f4b6 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -923,7 +923,19 @@ load-user-init-file
               ;; user-init-file conclusively.  Don't let it be
               ;; set from default.el.
               (when (eq user-init-file t)
-                (setq user-init-file init-file-name)))
+                (setq user-init-file
+                      ;; If the init-file doesn't exist, default to
+                      ;; the .el version, not the bare filename.  But
+                      ;; for backwards compatibility, don't do this if
+                      ;; we are using the alternate filename argument
+                      ;; (so we use ~/.emacs instead of ~/.emacs.el,
+                      ;; but ~/.emacs.d/early-init.el instead of
+                      ;; ~/.emacs.d/early-init, by default).
+                      (if alternate-filename-function
+                          init-file-name
+                        (concat
+                         (file-name-sans-extension init-file-name)
+                         ".el")))))
 
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-13  2:36                       ` Radon Rosborough
@ 2019-02-13 17:56                         ` Eli Zaretskii
  2019-02-16  0:47                           ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-13 17:56 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Tue, 12 Feb 2019 18:36:20 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > if you can come up with such a safe change, I will of course gladly
> > accept it.
> 
> It is attached.

Thanks.  This still uses the indirect evidence of the second function
argument.

Maybe all of this will be much simpler if in this fragment:

  ;; Load the early init file, if found.
  (load-user-init-file
   (lambda ()
     (expand-file-name
      "early-init"
      (file-name-as-directory
       (concat "~" init-file-user "/.emacs.d")))))

we replace "early-init" with "early-init.el"?  Does that solve the
problem?  If so, do you see any downsides with such a change?  We
didn't advertise the support for "~/.emacs.d/early-init", with no
extension.

WDYT?



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-13 17:56                         ` Eli Zaretskii
@ 2019-02-16  0:47                           ` Radon Rosborough
  2019-02-16  7:17                             ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Radon Rosborough @ 2019-02-16  0:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> Does that solve the problem?

Yes.

> If so, do you see any downsides with such a change?

Yes -- it lacks consistency and is therefore confusing -- but arguing
about this is tiring. Let's just do it that way. It's better than
leaving things as they are now.

Okay?

Best,
Radon

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-16  0:47                           ` Radon Rosborough
@ 2019-02-16  7:17                             ` Eli Zaretskii
  2019-02-16 20:26                               ` Radon Rosborough
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-02-16  7:17 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Fri, 15 Feb 2019 16:47:27 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > Does that solve the problem?
> 
> Yes.
> 
> > If so, do you see any downsides with such a change?
> 
> Yes -- it lacks consistency and is therefore confusing

I added a comment explaining why we do that, to hopefully alleviate at
least some of that confusion.

>  but arguing about this is tiring. Let's just do it that way. It's
> better than leaving things as they are now.
> 
> Okay?

Done.  Thanks for the feedback, and for reporting the problem to begin
with.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Fix `early-init-file' value when file is missing
  2019-02-16  7:17                             ` Eli Zaretskii
@ 2019-02-16 20:26                               ` Radon Rosborough
  0 siblings, 0 replies; 19+ messages in thread
From: Radon Rosborough @ 2019-02-16 20:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> Done. Thanks for the feedback, and for reporting the problem to begin
> with.

Much appreciated.

Best,
Radon

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-02-16 20:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-16  5:54 [PATCH] Fix `early-init-file' value when file is missing Radon Rosborough
2019-01-23  3:47 ` Radon Rosborough
2019-01-29 17:24   ` Radon Rosborough
2019-01-29 17:46     ` Eli Zaretskii
2019-02-01  9:11     ` Eli Zaretskii
2019-02-01 23:10       ` Radon Rosborough
2019-02-08  7:32         ` Eli Zaretskii
2019-02-08 17:34           ` Radon Rosborough
2019-02-08 21:52             ` Eli Zaretskii
2019-02-10 23:04               ` Radon Rosborough
2019-02-10 23:14                 ` Stefan Monnier
2019-02-11 16:05                 ` Eli Zaretskii
2019-02-12  5:38                   ` Radon Rosborough
2019-02-12 16:14                     ` Eli Zaretskii
2019-02-13  2:36                       ` Radon Rosborough
2019-02-13 17:56                         ` Eli Zaretskii
2019-02-16  0:47                           ` Radon Rosborough
2019-02-16  7:17                             ` Eli Zaretskii
2019-02-16 20:26                               ` Radon Rosborough

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).