When require-match argument is passed, an extra "" is added to completions

[vspinu]

(completing-read "sdfsd" '("a" "b") nil t) under ido-ubiquitous-mode adds an extra empty string to the proposed completions.

---

GNU Emacs 27.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.22.30) of 2019-03-15

;; Version: 4.12 ;; Package-Version: 20190502.2120

[DarwinAwardWinner]

This is by design. See here.

[DarwinAwardWinner]

If you have a specific command in mind that is broken by this behavior, let me know and I can add it to the default value of ido-cr+-nil-def-alternate-behavior-list.

[vspinu]

So you are overwriting the intend of the original author by adding an extra option, right? To my mind, if the caller specifies require-match it assumes that there is no default .Nil for default, means that there is no default, not that the default is "". If bare-bones completing-read pops "" on RET that's probably a bug and should be brought to emacs folks. AFAIC a real fix would be not to allow RET (C-j) as part of ido-cr+- and have a blacklists for all commands which want "" as an option, not the other way around. Every single command I know which uses require-match does NOT want "" as an option.

More importantly, why is this "feature" pops on require-match and not otherwise? When require-match is nil, bare-bones completion-read returns "" as well. So why don't I see this on (completing-read "prompt: " '("a" "b")) then?

In any case, this probably should not be "fixed" in the first place. In those extremely rare cases when "" is necessary and the author of the command didn't explicitly list "" as an option, the user can always pres "C-j" and get the desired "".

[DarwinAwardWinner]

I'm not overwriting anyone's intent, and I'm not replicating a bug in completing-read-default. The default value for DEF is documented to be the empty string in the docstring for completing-read:

If the input is null, completing-read returns DEF, or the first element of the list of default values, or an empty string if DEF is nil, regardless of the value of REQUIRE-MATCH.

There are functions in the wild that rely on this behavior, or else I wouldn't have implemented it in the first place. This has already been repeatedly discussed to death, most recently 2 years ago here, which led to the current implementation. Unless something significant has changed in the elisp ecosystem since then, I'm going to stick to current design of matching the documented behavior of completing-read by default and requiring an exception for callers that expect completing-read to work differently than documented. If you want ido-cr+ to disallow the empty string when require-match is non-nil, you should be able to do so by adding ".*" to ido-cr+-nil-def-alternate-behavior-list.

why is this "feature" pops on require-match and not otherwise?

Because when require-match is nil, you can already enter the empty string with C-j just like any other string, so no special handling is required.

[vspinu]

The default value for DEF is documented to be the empty string in the docstring for completing-read:

Arh, you are right. I should have read the completing-read doc carefully first before assuming I know how it works :(

you should be able to do so by adding ".*" to ido-cr+-nil-def-alternate-behavior-list.

Great. This will do for me as a user. But for code I would need to always provide the default in order to avoid explicit "" showing in the ido-cr+ completions, right? And I cannot effectively inhibit "" on C-j anyhow, or can I?

It's not feasible to track the intent of every single package out there. My feeling is that users, in order to avoid the annoyance, will be sticking ".*" into that list anyhow. Thus,essentially rendering it as a non-feature.

you can already enter the empty string with C-j just like any other string,

But that's the case for non-nil REQUIRE-MATCH as well. One can always insert an empty string with C-j no matter what. So, there is no strict need for an explicit "" in the list. Those function which rely on "" behavior are in small minority (my bet).

most recently 2 years ago here,

I see. Yet another occurrence of a "muddled-by-one-guy" discussion which didn't result in a constructive solution. What a pity.

[DarwinAwardWinner]

But for code I would need to always provide the default in order to avoid explicit "" showing in the ido-cr+ completions, right?

Yes, just like with completing-read-default, the default value of def is "". If you want something different, you have to specify it.

It's not feasible to track the intent of every single package out there.

As I said in the linked comment, tracking the intent of every single package out there is the only possible solution that works for everything. To do this, I could have implemented either a whitelist of packages that use completing-read correctly or a blacklist of packages that use it incorrectly. I chose to implement the latter, as explained above: I'd rather have the default behavior match the documented interface of completing-read.

One can always insert an empty string with C-j no matter what.

This isn't the case for ido-cr+. When require-match is non-nil, ido-cr+ requires that the returned value must be an element of collection. So the default always has to be added to the collection, even if it's the empty string (or nil, which is treated as equivalent to the empty string). This also means that if you specify a non-empty default and require a match, the user cannot exit ido-cr+ with the empty string. Again, this matches the behavior of completing-read-default, which also doesn't let you pick the empty string in this situation.

[vspinu]

This isn't the case for ido-cr+.

I see. I think I have now a pretty good understanding of how hairy the situation is. Thanks.

[vspinu]

I will stay with the default for a bit and will post here functions which I believe should be added as exceptions. For now, I think all magit commands should be added. On push I am seeing two candidates {"" "origin"}.

[DarwinAwardWinner]

Magit is a bit of a special case. Ido-cr+ doesn't mess with completing-read at all for magit. Instead, you have to set magit-completing-read-function to ido-completing-read+ (or rather the magit wrapper magit-ido-completing-read, whose only extra functionality is to fall back to regular completion if ido-cr+ can't be loaded).

Regardless, if you do this, then ido-completing-read+ is still getting called with the same arguments that would have been passed to completing-read, so the situation is very similar to what you'd get if it was actually being called from completing-read, and that means it does potentially make sense to detect when ido-cr+ is called from magit and omit the empty string option.

However, I'm not convinced this isn't subverting the intent of the magit author. For example, in the case of magit-push-current (the function that is called by the "push elsewhere" action that prompts for a remote to push to), it explicitly passes nil as the default. It's possible this is intentional, requiring you to type at least one letter (or select an option by cycling) before pressing RET. If it is intentional, then changing the behavior here would be overriding the magit author's intent for how this completion should work, and I'd rather not do that by default. Regardless, if you want to do it in your settings, this should be possible by adding magit-ido-completing-read to ido-cr+-nil-def-alternate-behavior-list.

In any case, magit is an actively developed package, so if you think it is failing to specify a useful default in cases where it could do so, you should file an issue with them. If it's fixed on magit's side, it will be fixed for all completion methods, not just ido.

If you find any other functions/packages that need this treatment, please create new issues for them. One issue per package is fine, although you can also report multiple packages in a single issue if you find several at once. Again, though, for actively developed packages, the preferred solution is always to get them to call completing-read correctly, i.e. not leaving DEF as nil when the empty string is not a valid completion. Ideally the overrides should only be used for old code that is unlikely to be fixed.

[vspinu]

Ok, clear. But gosh ... I wish I wouldn't have to deal with all that just for the sake of removing one "".

In any case, magit is an actively developed package, so if you think it is failing to specify a useful default in cases where it could do so, you should file an issue with them

There is no one else more familiar with the intricacies of completing read than yourself to file an issue. I am not confident enough to bring it up, nor, I bet, anyone else cursory annoyed by this issue.

it will be fixed for all completion methods, not just ido.

Others are probably not that much into theoretical perfection as you are. I don't recall helm or ivy popping and empty string by default. While I do see all the arguments, I am still far from convinced that damaging the user experience was worth it. It's just one extra push for people to switch to other methods.

[DarwinAwardWinner]

Ok, it seems that magit-completing-read always throws a user-error if REQUIRE-MATCH is non-nil and the completion function returns an empty string:

If REQUIRE-MATCH is non-nil and the users exits without a choice, an user-error is raised.

So that means that for magit-completing-read, REQUIRE-MATCH actually does mean a match is required. The correct fix for this is to modify magit-ido-completing-read to reflect this, so I'll file an issue to change that. (Edit: Here it is.)

Putting aside the special case of magit, the goal of ido-cr+ is to provide ido-based completion that is a perfect drop-in replacement for completing-read-default. That is, it should always be possible to replace any call to completing-read with a call to ido-completing-read+ without making any further changes and have it work. This means that the default settings must favor compatibility even when that requires a minor sacrifice in usability in some cases. I've done my best to provide enough customization that users who prefer a different trade-off can change it.