GNU bug report logs - #74208
31.0.50; minibuffer read-file-name-default mutates global value of default-directory incorrectly

Previous Next

Full log

View this message in rfc822 format

From: Madhu <enometh <at> meer.net>
To: monnier <at> iro.umontreal.ca
Cc: 74208 <at> debbugs.gnu.org
Subject: bug#74208: 31.0.50; minibuffer read-file-name-default mutates global value of default-directory incorrectly
Date: Sun, 10 Nov 2024 06:27:00 +0530 (IST)

[Message part 1 (text/plain, inline)]

[snip: I hope the context can be seen on the bug-list]
*  Stefan Monnier  <jwvjzdcz9fp.fsf-monnier+emacs <at> gnu.org>
> IIRC the `setq` is really there to set the value of `default-directory`
> during the whole duration of the completion, not just during the setup hook.
> This is needed/used if the user modifies the minibuffer's content to
> hold only a relative file name, at which point we need that default
> directory info in order to perform completions.

Ah yes I missed that, (though I haven't worked out yet how it works)

> The let+setq could be simplified to
>
>                         (let ((default-directory dir))
>
> but none of the code within this `let` uses `default-directory` since
> there are only calls to `car/cdr`, `setq`, and `set-syntax-table`, none
> of which touch file names, so we may as well remove the
> (setq default-directory dir) instead.

I still think the default-difectory should be let-bound so the binding
is undone when the dynamic scope exits.  I have not worked out where
the default-directory is being restored to the original value, but i
know it is possible for the global value to be set incorrectly
(e.g. after a M-: (debug), quit - -in the recursive minibuffer which
would do a non-local exit.)


> I'm leaning towards declaring the `shell-command` behavior above as
> a feature rather than a bug.

We could avoid still binding default-directory to a
non-directory. Using the logic of expand-file-name, how about the
following:

[shouldn't binding it before the call to `minibuffer-with-setup-hook'
be equivalent to a top level let in a hypothetical
(with-minibuffer-setup-hook FN &body BODY), or setting it via FN]

[0001-minibuffer.el-read-file-name-default-avoid-setting-i.patch (text/x-patch, inline)]

From ff4a2e046a66f7b364ad570c7dce11dab4987155 Mon Sep 17 00:00:00 2001
From: Madhu <enometh <at> net.meer>
Date: Tue, 5 Nov 2024 07:19:55 +0530
Subject: [PATCH] minibuffer.el: (read-file-name-default): avoid setting
 incorrect default-directory

* minibuffer.el: (read-file-name-default): bind default-directory
(in dynamic scope) instead of modifying it. try to bind it to a valid
directory. (e.g. don't set it to a url when calling (ffap) at a url).
---
 lisp/minibuffer.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index c970752ec2a..672f7b2b318 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3668,10 +3668,11 @@ read-file-name-default
                   ;; changing `default-directory' in the current buffer,
                   ;; we don't let-bind it.
                   (let ((dir (file-name-as-directory
-                              (expand-file-name dir))))
+                              (expand-file-name dir)))
+                        (default-directory (if (file-name-absolute-p dir)
+                                               dir default-directory)))
                     (minibuffer-with-setup-hook
                         (lambda ()
-                          (setq default-directory dir)
                           ;; When the first default in `minibuffer-default'
                           ;; duplicates initial input `insdef',
                           ;; reset `minibuffer-default' to nil.
-- 
2.46.0.27.gfa3b914457

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.