Re: find-file-read-args

[Juri Linkov]

> I think this is a useful feature.  I'm not yet 100% sure this is the
> best way to provide it (I think a dedicated key would be at least as
> good), but it saves us from figuring out which key to use: it's
> very unintrusive.
>
> So, I like it.  But I have some more questions:
>
>> --- 1275,1280 ----
>> ***************
>> *** 1298,1309 ****
>>       (remove-hook 'minibuffer-setup-hook ,hook)))))
>
>>   (defun find-file-read-args (prompt mustmatch)
>> !   (list (let ((find-file-default
>> !           (and buffer-file-name
>> !                (abbreviate-file-name buffer-file-name))))
>> !      (minibuffer-with-setup-hook
>> !          (lambda () (setq minibuffer-default find-file-default))
>> !        (read-file-name prompt nil default-directory mustmatch)))
>>      t))
>
>>   (defun find-file (filename &optional wildcards)
>> --- 1295,1301 ----
>>       (remove-hook 'minibuffer-setup-hook ,hook)))))
>
>>   (defun find-file-read-args (prompt mustmatch)
>> !   (list (read-file-name prompt nil default-directory mustmatch)
>>      t))
>
>>   (defun find-file (filename &optional wildcards)
>
> Can you explain this hunk?  This is a fairly delicate part of the
> behavior of C-x C-f,

The patch just moves the `minibuffer-with-setup-hook' from
`find-file-read-args' to `read-file-name', thus providing the logic of
setting of `minibuffer-default' to a wider set of file-reading
functions than only find-file-*.

> so I'd like to understand how you reproduce it.

This can be reproduced by typing M-n in the minibuffer of commands
other than find-file-*, e.g. `insert-file', `append-to-file',
`write-file', `write-region', `load-file', `recode-file-name',
`make-directory', `delete-directory', `copy-directory'.

>> +       ((eq major-mode 'dired-mode)
>> +        (let ((filename (dired-get-filename nil t)))
>> +          (when filename
>> +            (if (file-directory-p filename)
>> +                (file-name-as-directory (abbreviate-file-name filename))
>> +              (abbreviate-file-name filename)))))))
>
> This is ugly.  Why does it have to be here rather than somewhere in dired?

I can't find a function in dired.el and dired-aux.el that uses
`abbreviate-file-name' necessary for the minibuffer reading a file name.

>> +    (filename-at-point
>> +     (cond
>> +      ((fboundp 'ffap-guesser)
>> +       ;; Logic from `ffap-read-file-or-url' and `dired-at-point-prompter'
>> +       (let ((guess (ffap-guesser)))
>> +         (setq guess
>> +               (if (or (not guess)
>> +                       (and (fboundp 'ffap-url-p)
>> +                            (ffap-url-p guess))
>> +                       (and (fboundp 'ffap-file-remote-p)
>> +                            (ffap-file-remote-p guess)))
>> +                   guess
>> +                 (abbreviate-file-name (expand-file-name guess))))
>> +         (when guess
>> +           (if (file-directory-p guess)
>> +               (file-name-as-directory guess)
>> +             guess))))
>> +      ;; ((fboundp 'thing-at-point)
>> +      ;;  (thing-at-point 'filename))
>> +      )))
>
> I think this should be moved to a separate function (e.g. so it can be
> used by a separate key-binding) in file.el.  That function could/should
> obey a new hook file-name-at-point-functions hook which we'd run with
> run-hook-with-args-until-success.

OK, I'll create a new function that could later be bound to a dedicated key.

>> !                        ;; Unless a list of defaults in `minibuffer-default'
>> !                        ;; is provided, reset it to nil and on the
>> !                        ;; first request on `M-n' fill it with a list
>> !                        ;; of defaults relevant for file-name reading.
>> !                        (unless (consp minibuffer-default)
>> !                          (setq minibuffer-default nil)
>> !                          (set (make-local-variable 
>> 'minibuffer-default-add-function)
>> !                               (lambda ()
>> !                                 (with-current-buffer
>> !                                     (window-buffer 
>> (minibuffer-selected-window))
>> !                                   (read-file-name-defaults dir 
>> initial))))))
>
> Is it really necessary to throw away the minibuffer-default if it's not
> a cons?  I'd rather keep it and just add file-name-at-point.  That would
> make the change a lot more "obviously safe".

For most file/directory reading functions the minibuffer-default
duplicates the initial input, so the first typing M-n is useless since
it doesn't change the minibuffer's contents.  I'll try to better detect
this situation than checking for a cons.

>> Index: lisp/dired-aux.el
>
> Am I right that this part of the change is not strictly necessary?

dired-aux.el is from other threads with the subject "dired-dwim-target"
and "dired-dwim-target-directory" that has separate reasonings for separate
but related changes.  I'll send a new patch for dired-aux.el in another message.

-- 
Juri Linkov
http://www.jurta.org/emacs/