Update our completing-read-multiple replacement/wrapper

Among other things we use crm to read revision ranges such as
"A..B" and "C...D".  As far as crm is concerned ".." and "..." are
interchangeable separators but to Git they mean different things.
We cannot use `completing-read-multiple' when reading ranges because
it returns a list of the read values, stripping the separators.

`magit-completing-read-multiple' was added in [1: 388a9a254] to avoid
some (additional upcoming) duplication between our two functions that
read ranges.

Giving it that name was a mistake because its function signature
is not compatible with that of `completing-read-multiple'.  In
[2: cb4d59190] `magit-completing-read-multiple*' was added to fix
that, but we did not port existing callers immediately because of
the involved risk.

Meanwhile we added more kludges to `magit-completing-read-multiple'
and `magit-completing-read-multiple*', addressing additional defects
and undesirable features of various completion frameworks.

Now the time has come to deprecate `magit-completing-read-multiple'
in favor of `magit-completing-read-multiple*'.  We should stop using
the former name for now, because of the differences in the function
signatures.  Eventually we would like to use the former name again,
but if we abruptly changing the signature of an existing function,
that would risk breaking third-party packages.  The use of an
intermediate function name minimizes the risk to third-parties.

The previous approach was to copied `completing-read-multiple' and
to modified the copy to our needs, but we then failed to incorporate
upstream changes into our copies, so now we use a wrapper instead.

Another reason to use a wrapper is that then we don't bypass any
advice that may have been added to the original.  For example the
`vertico' completion framework advices `completing-read-multiple'
but that advise obviously did not affect our copies.

1: 388a9a254f6ee985e67926d44a2d9ebad79af897
   completion: move common crm setup to a function

2: cb4d591900187b77e54ec00cac5f25b644153b5b
   magit-completing-read-multiple*: New function

Because this commit shuffles things around quite a bit, making history
inspection harder, I have added a code comment for each issue that our
wrapper addresses.

Co-authored-by: Miha Rihtaršič <miha@kamnitnik.top>
- Avoid changing the return value of split-string as doing that
  would break selectrum's crm function.
- Cosmetics.

1 files changed, 4 insertions, 5 deletions

diff --git a/lisp/magit-push.el b/lisp/magit-push.el
index 2a98591..3710e4e 100644
--- a/lisp/magit-push.el
+++ b/lisp/magit-push.el
@@ -217,11 +217,10 @@ only available for the part before the colon, or when no colon
 is used."
   (interactive
    (list (magit-read-remote "Push to remote")
-         (split-string (magit-completing-read-multiple
-                        "Push refspec,s"
-                        (cons "HEAD" (magit-list-local-branch-names))
-                        nil nil 'magit-push-refspecs-history)
-                       crm-default-separator t)
+         (magit-completing-read-multiple*
+          "Push refspec,s: "
+          (cons "HEAD" (magit-list-local-branch-names))
+          nil nil nil 'magit-push-refspecs-history)
          (magit-push-arguments)))
   (run-hooks 'magit-credential-hook)
   (magit-run-git-async "push" "-v" args remote refspecs))