Re: Slight confusion in project-find-file

[Dmitry Gutov]

Hi!

On 13.09.2021 17:07, Lars Ingebrigtsen wrote:
> I was looking at
>
> *** 'project-find-file' doesn't use the string at point as default input.
> Now it's only suggested as part of the "future history".
>
> which comes from
>
> commit fa895379d6166c52b89774a9e450a94c2e429dae
> Author:     Dmitry Gutov <dgutov@yandex.ru>
> AuthorDate: Fri Aug 6 03:30:10 2021 +0300
>
>      Change how project-find-file's completion works
>      
>      * lisp/progmodes/project.el (project--completing-read-strict):
>      Allow to choose a non-existent file, with confirmation
>      (bug#49204).
>      Don't use the string at point as a "real" default, and instead
>      only include it in "future history": meaning, it will be inserted
>      on 'M-n' (bug#49865).
>
> And, indeed, it doesn't put anything in the default.  However, I'm
> getting a bit confused trying to follow the logic:
>
> (defun project-or-external-find-file ()
>    "Visit a file (with completion) in the current project or external roots.
>
> The completion default is the filename at point, determined by
> `thing-at-point' (whether such file exists or not)."
>
> So this says "default", but "completion default"...  (And same string in
> project-find-file).  Also:
>
> @findex project-find-file
> [...]
> belonging to the current project, and nothing else.  If there's a file
> name at point, this command offers that file as the default to visit.
>
> However, we basically end up calling:
>
> (uniq-file-read "Prompt" nil nil nil "file-name")
>
> there "file-name" is in the DEFAULT position, but is put in the "future
> history".

Indeed, there is a documentation problem. Part of it is that "future 
history" is not a well-known/defined concept, yet it is pretty useful.

Another - that future history is added to using a variable called 
minibuffer-default-add-function.

I've tried to improve the docs in commit 5a34b65a3b, not sure by how 
much. See how you like the result.

As far as using DEFAULT for default, it has been the previous behavior, 
and reasons for the change are hopefully apparent from the linked 
discussion. There have also been some other complaints/requests that 
have been resolved with that change.

> Furthermore, `M-x project-find-file RET RET':
>
> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>    expand-file-name(nil)
>    find-file-noselect(nil nil nil nil)
>    find-file(nil)
>    project-find-file-in(#("." 0 1 (fontified t)) ("~/src/emacs/trunk/") (vc . 
> "~/src/emacs/trunk/"))
>    project-find-file()
>    funcall-interactively(project-find-file)
>
> So I think this needs a bit more updating in the doc
> strings/documentation/parameter names, and perhaps a different way of
> erroring out on RET.  (Or even actually using the DEFAULT as a default.)

This is really a bug of completing-read: returning "" on RET even if 
REQUIRE-MATCH is t. Probably a "feature" since the bug is decades old.

I've made it loop on RET instead now, hopefully it will resolve this 
scenario. FWIW, I couldn't repro the wrong-type-argument error here, but 
the behavior was surprising nevertheless.