In eshell's completion-at-point, consult-completion-in-region places the minibuffer point in the middle of the completion string

[tomfitzhenry]

Steps to reproduce

GNU Emacs 27.1
consult: b5cbc61

$ emacs -q
(require 'consult)
(setq completion-in-region-function 'consult-completion-in-region)

In eshell, run:

touch foobar
touch football
cat foo<TAB>

Expected

I'm prompted "Completion: foo|" where | indicates where I expect point to be.

Actual

I'm prompted "Completion: |foo".

This is a problem because I then type "b" (to try to complete "foob" to "foobar"), the minibuffer actually shows "bfoo" which fails to complete.

[tomfitzhenry]

Workaround: Remove this clause:

consult/consult.el

Lines 627 to 632 in b5cbc61

	(if (eq category 'file)
	(read-file-name "Completion: "
	(file-name-directory initial)
	initial t
	(file-name-nondirectory initial)
	predicate)

More practical workaround:

(add-hook 'eshell-mode-hook
          (lambda ()
            (setq-local completion-in-region-function 'completion--in-region)))

[minad]

@oantolin ideas?

[oantolin]

Huh. Like @tomfitzhenry pointed out, it's read-file-name that's doing that! I don't know why, but it chooses to put point before the argument initial rather than at the end of the minibuffer. That doesn't seem like a very good default. We could just not use read-file-name, but it is nice to have your special keybindings from minibuffer-local-filename-completion-map present.

How about just forcing the move to the end of line? We can wrap the call to read-file-name in (minibuffer-with-setup-hook #'end-of-line _____).

[minad]

@tomfitzhenry are you using selectrum or icomplete btw?

[tomfitzhenry]

@tomfitzhenry are you using selectrum or icomplete btw?

I can reproduce this issue with emacs -q (so not even icomplete is enabled), and with icomplete. I haven't tried selectrum.

[oantolin]

It's definitely something that read-file-name-default does. Selectrum uses its own setting for read-file-name-function, so it's possible it behaves differently.

[tomfitzhenry]

Huh. Like @tomfitzhenry pointed out, it's read-file-name that's doing that! I don't know why, but it chooses to put point before the argument initial rather than at the end of the minibuffer. That doesn't seem like a very good default.

I'll try to reproduce this using just read-file-name, and raise that issue upstream. Working around bugs long-term isn't sustainable.

[minad]

If it is not our bug, I would not fix it here. As long as we are sure we did everything alright.

[tomfitzhenry]

The steps to reproduce are simple. I'll raise this upstream.

(read-file-name-default "Read File: " nil nil nil "Initial" nil)

[minad]

Regarding the initial value - this has been deprecated for the completing-read API, but for the read-file-name API it is fine? The docs only say "Please note: we recommend using default rather than initial in most cases". See https://www.gnu.org/software/emacs/manual/html_node/elisp/Reading-File-Names.html.

[tomfitzhenry]

Further, that page says "If you specify initial, that is an initial file name to insert in the buffer (after directory, if that is inserted). In this case, point goes at the beginning of initial."

That is, the behaviour we observe is the expected behaviour. I can't tell why that is desirable behaviour (even given the described example of find-alternate-file).

I won't raise this upstream: it's working as intended.

Our options:
i. don't use read-file-name
ii. figure out a way to do this via default. I can't see how we would. default appears to be used for when selection fails, not as a means of pre-populating the minibuffer UI (it doesn't).
iii. It looks like we can include the file name in the dir argument: (read-file-name-default "Read File: " "~/foo" nil nil nil nil) does put point where we expect, and matches file "~/foobar". This appears to be undefined behaviour.
iv. Advise, as in #48 (comment)

[tomfitzhenry]

I lean towards (i) since:

1. It's cleanest.
2. It's easier to add read-file-name later (if we figure out a clean way to do it), than to remove it later when people may depend on it, if supporting this unclean solution proves too burdensome.

it is nice to have your special keybindings from minibuffer-local-filename-completion-map present

How important is this usecase? Are there file-specific bindings that people would want during a completion-at-point?

[minad]

Why not raise it upstream, if you think it is bad behavior? Option (i) would revert the special behavior for files, which has been done for consistency with how selectrum. It and it also makes sense to use read-file for the file category, I think. But I was not really involved the design of this function, @oantolin made it and we got some feedback by @clemera.

[tomfitzhenry]

I think I've found a way to avoid using read-file-name, but keep minibuffer-local-filename-completion-map bindings. It looks like we can set minibuffer-completing-file-name to t to tell the minibuffer to treat this as a filename.

Update: This works, but prompts with a relative path (and so doesn't look like a prompt for a filename). Try to make that an absolute prompt, and then results are absolute. Fixing that partly amounts to reimplementing read-file-name!

[minad]

@tomfitzhenry Since this already has issues and it won't work nicely with recursive minibuffer I think it would be better to keep using read-file-name or not use it at all. But this has to be discussed. See #31 for the original discussion.

[oantolin]

Personally, I'd go with @tomfitzhenry's option (iii), use the directory argument: even if this behavior is not documented it seems unlikely to change.

[minad]

Haha, the undefined behavior. I am fine with it as long as it works. If at some point it stops working we can file a bugreport to upstream 🙈

[oantolin]

Exploiting the undefined behavior seems to work just fine, and it really isn't likely to change any time soon. I'll make a pull request.

[minad]

Closed by #63