bug#19068: Mail file vars aren't derived from customized message-directory

bug#19068: Mail file vars aren't derived from customized message-directory

[Kelly Dean]

Delete your ~/Mail directory (uppercase ⌜M⌝).
Make a ~/mail directory (lowercase ⌜m⌝).

Put in your init file just:
(require 'message)
(setq message-directory "~/mail/")

Start Emacs 24.4.
Notice that message-auto-save-directory is now ⌜~/⌝.
The default value of message-auto-save-directory is
  (if (file-writable-p message-directory)
      (file-name-as-directory (expand-file-name "drafts" message-directory))
    "~/")
but that fails to work as intended, because message-directory still has its default value of ⌜~/Mail/⌝, because message.el hasn't been loaded yet.

The docstring for message-directory says ⌜Directory from which all other mail file variables are derived⌝, which is misleading because it implies that if you customize that variable, all other mail file variables will be changed to match your customization.

You could avoid the problem by ensuring that the setq comes not only before the require of message, but also before the require of anything else that might require message, but that defeats the point of require (which is supposed to avoid the brittleness of load).

Fixing this requires either the other vars to be changed into functions that dynamically derive pathnames from message-directory, or something like a set-message-directory function to be made as a replacement for the message-directory var, to update all the other vars when it's called.

Also,
grep -r "~/Mail/" emacs-24.4/lisp/ | grep 'el:'
gives 19 hits, all of which are inappropriate if ~/Mail isn't supposed to be hardcoded. If ~/Mail is supposed to be hardcoded, then the message-directory variable should be removed, to avoid misleading users.

bug#19068: Mail file vars aren't derived from customized message-directory

[Kelly Dean]

I wrote:
> that fails to work as intended, because message-directory still has its
> default value of ⌜~/Mail/⌝, because message.el hasn't been loaded yet.
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I meant:
...because the setq in your init file hasn't been reached yet.

bug#19068: [PATCH] Mail file vars aren't derived from customized message-directory

[Kelly Dean]

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

The attached patch fixes this bug.

This patch relies on the varhook feature. For details, see:
https://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00974.html


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: message-directory-bug.patch --]
[-- Type: text/x-diff, Size: 1000 bytes --]

--- emacs-24.4/lisp/gnus/message.el
+++ emacs-24.4/lisp/gnus/message.el
@@ -120,6 +120,9 @@
   :group 'message-various
   :type 'directory)
 
+(defvar message-directory-varhook nil)
+(put 'message-directory 'varhook 'message-directory-varhook)
+
 (defcustom message-max-buffers 10
   "*How many buffers to keep before starting to kill them off."
   :group 'message-buffers
@@ -1326,6 +1329,16 @@
   :link '(custom-manual "(message)Various Message Variables")
   :type '(choice directory (const :tag "Don't auto-save" nil)))
 
+;; Update message-auto-save-directory when message-directory changes.
+;; Fixes bug #19068.
+(add-hook 'message-directory-varhook
+	  (lambda (_sym _env)
+	    (setq message-auto-save-directory
+		  (if (file-writable-p message-directory)
+		      (file-name-as-directory
+		       (expand-file-name "drafts" message-directory))
+		    "~/"))))
+
 (defcustom message-default-charset
   (and (not (mm-multibyte-p)) 'iso-8859-1)
   "Default charset used in non-MULE Emacsen.

bug#19068: Mail file vars aren't derived from customized message-directory

[Lars Ingebrigtsen]

"Kelly Dean" <kelly@prtime.org> writes:

> Delete your ~/Mail directory (uppercase ⌜M⌝).
> Make a ~/mail directory (lowercase ⌜m⌝).
>
> Put in your init file just:
> (require 'message)
> (setq message-directory "~/mail/")

That's the wrong way to set variables that have other variables that
depend on them.

Instead say

(setq message-directory "~/mail/")
(require 'message)

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#19068: Mail file vars aren't derived from customized message-directory

[Ivan Shmakov]

>>>>> Kelly Dean <kelly@prtime.org> writes:
>>>>> Lars Ingebrigtsen wrote:

	[Cc: 19068@, as the discussion still seems relevant to the bug.]

 >> That's the wrong way to set variables that have other variables that
 >> depend on them.

 >> Instead say

 >> (setq message-directory "~/mail/") (require 'message)

 > And what if you happened to previously require something that already
 > required message?  Do you want to require users to always put all
 > their «setq»s before all their «require»s, just in case?

	As long as we speak about ~/.emacs, my free advice to the users
	would be to ‘setq’ first, and ‘require’ never.

	That is: if the user needs an explicit ‘require’ there, it’s
	quite likely that something is already broken.  Normally, all
	the Emacs packages’ “entry points” are autoloaded, and enabling
	a particular function should be just a matter of setting up a
	specific hook, or adding an entry to a specific alist, etc.

	In the worst case, the user may need to add an ‘autoload’ form
	to his or her own ~/.emacs, if one’s somehow missing from the
	package’s own .el (or loaddefs.el, or the user’s own private
	analogue thereof.)

 > Or what if you were already using message mode with the default
 > directory settings, but then you decided to change it and customize
 > message-directory using Emacs's customization feature, and read the
 > help page that says ⌜Directory from which all other mail file
 > variables are derived⌝?

	I agree that the docstring for this variable is misleading, – it
	is /not/ the usual semantics for a variable to change when some
	other variable (however related) is changed, – neither in
	Emacs Lisp, nor in the majority of the programming languages
	I know.  (One notable exception being Make.)

	That’s contrary to, say, Gnus “group parameters,” which are
	reconsidered something like every time the group is accessed.
	I guess it’s possible to reimplement message-directory and its
	“dependent” variables in a similar manner, but I doubt it’d
	worth the effort.

 > Would you not expect that when you change a top-level directory, the
 > directories under it remain under it?  After all, that's the way «mv»
 > behaves.

	To continue with the analogy, if you $ dir=~/mail in the shell,
	and then $ mv ~/mail ~/othername, would you expect for ${dir} to
	still refer to the same directory, – now known as ~/othername?

-- 
FSF associate member #7257  np. Omega — Bruce Dickinson … 3013 B6A0 230E 334A

bug#19068: Mail file vars aren't derived from customized message-directory

[Eli Zaretskii]

> From: "Kelly Dean" <kelly@prtime.org>
> Date: Sun, 16 Nov 2014 11:27:23 +0000
> 
> Also,
> grep -r "~/Mail/" emacs-24.4/lisp/ | grep 'el:'
> gives 19 hits, all of which are inappropriate if ~/Mail isn't supposed to be hardcoded. If ~/Mail is supposed to be hardcoded, then the message-directory variable should be removed, to avoid misleading users.

This is a red herring: all of these hits are either in comments or in
default values of other defcustoms.

bug#19068: Mail file vars aren't derived from customized message-directory

[Kelly Dean]

Eli Zaretskii wrote:
>> Also,
>> grep -r "~/Mail/" emacs-24.4/lisp/ | grep 'el:'
>> gives 19 hits, all of which are inappropriate if ~/Mail isn't supposed to be hardcoded. If ~/Mail is supposed to be hardcoded, then the message-directory variable should be removed, to avoid misleading users.
>
> This is a red herring: all of these hits are either in comments or in
> default values of other defcustoms.

If a user renames his ⌜Mail⌝ directory to ⌜mail⌝, then he'll want all the things that previously used ⌜Mail⌝ to use ⌜mail⌝. Manually changing them all is tedious and error-prone, so it'd be nice to have one place to make the change. In Emacs, message-directory advertises itself as that place.

I lowercased my mail directory name both because it's easier to type that way (don't have to press shift), and because some other things already use the lowercase version by default, and there was no reason to use both upper- and lower-case versions.

bug#19068: Mail file vars aren't derived from customized message-directory

[Ivan Shmakov]

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

>>>>> Kelly Dean <kelly@prtime.org> writes:
>>>>> Eli Zaretskii wrote:

 >>> Also, grep -r "~/Mail/" emacs-24.4/lisp/ | grep 'el:' gives 19
 >>> hits, all of which are inappropriate if ~/Mail isn't supposed to be
 >>> hardcoded.  If ~/Mail is supposed to be hardcoded, then the
 >>> message-directory variable should be removed, to avoid misleading
 >>> users.

 >> This is a red herring: all of these hits are either in comments or
 >> in default values of other defcustoms.

 > If a user renames his ⌜Mail⌝ directory to ⌜mail⌝, then he'll want all
 > the things that previously used ⌜Mail⌝ to use ⌜mail⌝.  Manually
 > changing them all is tedious and error-prone, so it'd be nice to have
 > one place to make the change.  In Emacs, message-directory advertises
 > itself as that place.

	Only as long as message-mode (or anything deriving from it) is
	considered.

	I tend to think that defcustom’s :set does not fit for this
	case, and instead suggest using nil as the default for the
	variables whose defaults derive from message-directory, –
	something along the lines of the (untested) patch MIMEd.

	* lisp/gnus/message.el (subr-x): Require feature.
	(message-auto-save-directory): Default to nil.
	(message-auto-save-directory): New function.
	(message-set-auto-save-file-name): Use it.

 > I lowercased my mail directory name both because it's easier to type
 > that way (don't have to press shift),

	Seconded.

 > and because some other things already use the lowercase version by
 > default, and there was no reason to use both upper- and lower-case
 > versions.

-- 
FSF associate member #7257  np. Face Another Day — Jogeir Liljedahl 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 2362 bytes --]

--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -29,7 +29,8 @@
 ;;; Code:
 
 (eval-when-compile
-  (require 'cl))
+  (require 'cl)
+  (require 'subr-x))			; For when-let.
 
 (require 'mailheader)
 (require 'gmm-utils)
@@ -1331,12 +1332,10 @@
   :group 'message-various
   :type '(repeat function))
 
-(defcustom message-auto-save-directory
-  (if (file-writable-p message-directory)
-      (file-name-as-directory (expand-file-name "drafts" message-directory))
-    "~/")
+(defcustom message-auto-save-directory nil
   "*Directory where Message auto-saves buffers if Gnus isn't running.
 If nil, Message won't auto-save."
+  :version 25.1
   :group 'message-buffers
   :link '(custom-manual "(message)Various Message Variables")
   :type '(choice directory (const :tag "Don't auto-save" nil)))
@@ -6666,12 +6665,24 @@ defun message-setup-1 (headers &optional yank-action actions return-action)
   ;; rmail-start-mail expects message-mail to return t (Bug#9392)
   t)
 
+(defun message-auto-save-directory nil
+  "Return message auto save directory.
+Return the value of the `message-auto-save-directory' variable if non-nil.
+Otherwise, if `message-directory' is non-nil, return a suitable
+directory name under it if it is writeable, or "~/" if not.
+Return nil if all the above fails."
+  (cond ((not message-directory) nil)
+	((file-writable-p message-directory)
+	 (file-name-as-directory
+	  (expand-file-name "drafts" message-directory)))
+	(t "~/")))
+
 (defun message-set-auto-save-file-name ()
   "Associate the message buffer with a file in the drafts directory."
-  (when message-auto-save-directory
+  (when-let ((dir (message-auto-save-directory)))
     (unless (file-directory-p
-	     (directory-file-name message-auto-save-directory))
-      (make-directory message-auto-save-directory t))
+	     (directory-file-name dir))
+      (make-directory dir t))
     (if (gnus-alive-p)
 	(setq message-draft-article
 	      (nndraft-request-associate-buffer "drafts"))
@@ -6689,7 +6700,7 @@ defun message-set-auto-save-file-name ()
 				  "message"
 				"*message*")
 			       (format-time-string "-%Y%m%d-%H%M%S"))
-			      message-auto-save-directory))
+			      dir))
       (setq buffer-auto-save-file-name (make-auto-save-file-name)))
     (clear-visited-file-modtime)
     (setq buffer-file-coding-system message-draft-coding-system)))

bug#19068: Mail file vars aren't derived from customized message-directory

[Ivan Shmakov]

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

>>>>> Ivan Shmakov <ivan@siamics.net> writes:

[…]

 > I [hereby] suggest using nil as the default for the variables whose
 > defaults derive from message-directory, – something along the lines
 > of the (untested) patch MIMEd.

 > * lisp/gnus/message.el (subr-x): Require feature.
 > (message-auto-save-directory): Default to nil.

	This of course will not work, as this variable already uses nil
	to mean “do not auto save,” so we need some other value (say,
	'auto) for the purpose.

	Please consider the revised patch MIMEd.

	* lisp/gnus/message.el (subr-x): Require feature.
	(message-auto-save-directory): Default to 'auto.
	(message-auto-save-directory): New function.
	(message-set-auto-save-file-name): Use it.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 2799 bytes --]

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index de7e9ba..da5c871 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -29,7 +29,8 @@
 ;;; Code:
 
 (eval-when-compile
-  (require 'cl))
+  (require 'cl)
+  (require 'subr-x))			; For when-let.
 
 (require 'mailheader)
 (require 'gmm-utils)
@@ -1331,15 +1332,16 @@
   :group 'message-various
   :type '(repeat function))
 
-(defcustom message-auto-save-directory
-  (if (file-writable-p message-directory)
-      (file-name-as-directory (expand-file-name "drafts" message-directory))
-    "~/")
+(defcustom message-auto-save-directory 'auto
   "*Directory where Message auto-saves buffers if Gnus isn't running.
-If nil, Message won't auto-save."
+If nil, Message won't auto-save.
+If 'auto, derive from `message-directory'."
+  :version "25.1"
   :group 'message-buffers
   :link '(custom-manual "(message)Various Message Variables")
-  :type '(choice directory (const :tag "Don't auto-save" nil)))
+  :type '(choice directory
+		 (const :tag "Don't auto-save" nil)
+		 (const :tag "Derive from `message-directory'" auto)))
 
 (defcustom message-default-charset
   (and (not (mm-multibyte-p)) 'iso-8859-1)
@@ -6666,12 +6668,28 @@
   ;; rmail-start-mail expects message-mail to return t (Bug#9392)
   t)
 
+(defun message-auto-save-directory nil
+  "Return message auto save directory.
+Return the value of the `message-auto-save-directory' variable if it is
+a string or nil.
+Otherwise, if `message-directory' is non-nil, return a suitable
+directory name under it if it is writeable, or \"~/\" if not.
+Return nil if all the above fails."
+  (cond ((or (stringp message-auto-save-directory)
+	     (not message-auto-save-directory))
+	 message-auto-save-directory)
+	((not message-directory) nil)
+	((file-writable-p message-directory)
+	 (file-name-as-directory
+	  (expand-file-name "drafts" message-directory)))
+	(t "~/")))
+
 (defun message-set-auto-save-file-name ()
   "Associate the message buffer with a file in the drafts directory."
-  (when message-auto-save-directory
+  (when-let ((dir (message-auto-save-directory)))
     (unless (file-directory-p
-	     (directory-file-name message-auto-save-directory))
-      (make-directory message-auto-save-directory t))
+	     (directory-file-name dir))
+      (make-directory dir t))
     (if (gnus-alive-p)
 	(setq message-draft-article
 	      (nndraft-request-associate-buffer "drafts"))
@@ -6689,7 +6707,7 @@ defun message-set-auto-save-file-name ()
 				  "message"
 				"*message*")
 			       (format-time-string "-%Y%m%d-%H%M%S"))
-			      message-auto-save-directory))
+			      dir))
       (setq buffer-auto-save-file-name (make-auto-save-file-name)))
     (clear-visited-file-modtime)
     (setq buffer-file-coding-system message-draft-coding-system)))

bug#19068: Mail file vars aren't derived from customized message-directory

[Eli Zaretskii]

> From: Kelly Dean <kelly@prtime.org>
> CC: 19068@debbugs.gnu.org
> Date: Fri, 30 Jan 2015 07:11:52 +0000
> 
> Eli Zaretskii wrote:
> >> Also,
> >> grep -r "~/Mail/" emacs-24.4/lisp/ | grep 'el:'
> >> gives 19 hits, all of which are inappropriate if ~/Mail isn't supposed to be hardcoded. If ~/Mail is supposed to be hardcoded, then the message-directory variable should be removed, to avoid misleading users.
> >
> > This is a red herring: all of these hits are either in comments or in
> > default values of other defcustoms.
> 
> If a user renames his ⌜Mail⌝ directory to ⌜mail⌝, then he'll want all the things that previously used ⌜Mail⌝ to use ⌜mail⌝. Manually changing them all is tedious and error-prone, so it'd be nice to have one place to make the change. In Emacs, message-directory advertises itself as that place.

That's a separate issue.  I'm not at all sure the user will always
want to rename all of them, but we could offer an option to do that.

bug#19068: Mail file vars aren't derived from customized message-directory

[Lars Ingebrigtsen]

I don't think there's a bug here to fix, so I'm closing this report.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#19068: Mail file vars aren't derived from customized message-directory

[Ivan Shmakov]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:

 >> The docstring for message-directory says ⌜Directory from which all
 >> other mail file variables are derived⌝, which is misleading because
 >> it implies that if you customize that variable, all other mail file
 >> variables will be changed to match your customization.

[…]

 > I don't think there's a bug here to fix, so I'm closing this report.

	I tend to agree that the docstring is misleading; either it
	should be fixed to provide more detail on the current behavior
	(say: “Setting this variable after 'message is loaded has no
	effect on the derived variables, though.”), or the current
	behavior should be changed so that the “derived” variables
	remain based on this variable’s value, irrespective of when the
	latter is set.

 >> Fixing this requires either the other vars to be changed into
 >> functions that dynamically derive pathnames from message-directory,

	Which is what the patch I’ve suggested earlier [1] should do.

[1] news:877fw4jsal.fsf@violet.siamics.net
    http://debbugs.gnu.org/19068#msg32

[…]

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A