Improper use of require-match in completing-read functions

[DarwinAwardWinner]

I am the author of ido-compelting-read+, and I recently received a bug report about an interaction between ebal and the latest version of my package: DarwinAwardWinner/ido-completing-read-plus#127. The problem stems from the fact that the require-match argument to completing-read doesn't actually mean what it says. From the docstring of completing-read (emphasis mine):

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.

In other words, the default value for DEF is effectively "", which means that if you don't provide your own value for DEF, the empty string is considered a valid completion. From the issue referenced above and my reading of the code, this does not appear to be the intent for ebal. To solve this, you should pass (car collection) as the default to any completing-read-type function (or (car (all-completions "" collection)) if collection is not already a list.

In addition, I don't think there's any need for your package to provide explicit support for ido completion. If users set ebal-completing-read-function to the default of ebal-built-in-completing-read and enable ido-ubiquitous-mode, they will already get ido completion in ebal and everywhere else. The main reason to provide a separate ido-based completing-read function for your package would be if you think that people will want ido completion for your package but not anywhere else (or vice versa), which I think is unlikely, since people would generally either want it everywhere or nowhere for consistency.

[mrkkrp]

Thanks for opening the issue. I don't use IDO anymore, now I use ivy-mode. With it at least there is no difference between for example:

(funcall ebal-completing-read-function
         "Build target: "
         (cons "default" ebal--project-targets)
         nil
         t)

and

(funcall ebal-completing-read-function
                      "Build target: "
                      (cons "default" ebal--project-targets)
                      nil
                      t
                      nil
                      nil
                      "default")

In both cases I'm forced to select something.

And what on the earth does it mean "if the input is null"? Null is not nil and it's certainly not an empty string. I'm not sure what it is.

don't think there's any need for your package to provide explicit support for ido completion

Right, I'm actually dropping dependency on ido-completing-read+ right now, see #13.

[DarwinAwardWinner]

In this case, "null" input means the user has not entered any text, i.e. the empty string. You can test this for yourself. Run (completing-read-default "Pick a color: " '(red green blue) nil t) and press RET. It will return the empty string.

It's not surprising that you don't see an issue when using ivy, since ivy doesn't follow the documented behavior of completing-read on empty strings except for specific functions: abo-abo/swiper#1049

Also, in #13, rather than removing them, you might want to define ebal-ido-completing-read and ebal-command-ido as obsolete aliases to completing-read that generate a warning that you should enable ido-ubiquitous-mode for ido completion, so that people who were previously using these options are automatically offered an upgrade path.