Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

improve completion #1245

Merged
merged 5 commits into from

1 participant

@tarsius
Owner

Also see #1220.

tarsius added some commits
@tarsius tarsius magit-completing-read-function: sort choices by preference 26e7d30
@tarsius tarsius magit-{ido,iswitchb}-completion-read: doc-string cosmetics 50cb068
@tarsius tarsius magit-prompt-with-default: new function
Split `magit-prompt-with-default' from `magit-builtin-completing-read'
so that it could also be used by other `completing-read' replacements
and wrappers.
5b64196
@tarsius tarsius magit-completing-read: be strict about REQUIRE-MATCH
`completing-read's REQUIRE-MATCH means "require the user to exit with
a match, but there are exceptions in which case an empty string is
returned".  That's not so useful because every caller has to check
for that empty string.

Previously `magit-builtin-completing-read' did detect that case and
raise an error if `completing-read' returned an empty string.  Now
we do this in `magit-completing-read' so that this sanity check is
applied regardless of what `magit-completing-read-function' is used.

Remove equivalent checks from `magit-read-remote-branch' and
`magit-read-rev'.  Now that we do this in the correct place we no
longer have to do it in several places.
4c59d52
@tarsius tarsius magit-completing-read: better doc-string 05ea5db
@tarsius tarsius merged commit 05ea5db into next
@tarsius tarsius deleted the n/completion-1 branch
@tarsius tarsius added this to the 2.1.0 milestone
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 22, 2014
  1. @tarsius
  2. @tarsius
  3. @tarsius

    magit-prompt-with-default: new function

    tarsius authored
    Split `magit-prompt-with-default' from `magit-builtin-completing-read'
    so that it could also be used by other `completing-read' replacements
    and wrappers.
  4. @tarsius

    magit-completing-read: be strict about REQUIRE-MATCH

    tarsius authored
    `completing-read's REQUIRE-MATCH means "require the user to exit with
    a match, but there are exceptions in which case an empty string is
    returned".  That's not so useful because every caller has to check
    for that empty string.
    
    Previously `magit-builtin-completing-read' did detect that case and
    raise an error if `completing-read' returned an empty string.  Now
    we do this in `magit-completing-read' so that this sanity check is
    applied regardless of what `magit-completing-read-function' is used.
    
    Remove equivalent checks from `magit-read-remote-branch' and
    `magit-read-rev'.  Now that we do this in the correct place we no
    longer have to do it in several places.
  5. @tarsius
This page is out of date. Refresh to see the latest.
Showing with 55 additions and 35 deletions.
  1. +55 −35 magit.el
View
90 magit.el
@@ -540,9 +540,9 @@ large diffs. Also see option `magit-diff-use-overlays'."
(defcustom magit-completing-read-function 'magit-builtin-completing-read
"Function to be called when requesting input from the user."
:group 'magit
- :type '(radio (function-item magit-iswitchb-completing-read)
+ :type '(radio (function-item magit-builtin-completing-read)
(function-item magit-ido-completing-read)
- (function-item magit-builtin-completing-read)
+ (function-item magit-iswitchb-completing-read)
(function :tag "Other")))
(defcustom magit-repo-dirs nil
@@ -3739,31 +3739,51 @@ Return a list of two integers: (A>B B>A)."
(defun magit-completing-read
(prompt collection &optional predicate require-match initial-input hist def)
- "Call function in `magit-completing-read-function' to read user input.
+ "Magit wrapper around `completing-read' or an alternative function.
-Read `completing-read' documentation for the meaning of the argument."
- (funcall magit-completing-read-function
- (concat prompt ": ") collection predicate
- require-match initial-input hist def))
+Option `magit-completing-read-function' can be used to wrap
+around another `completing-read'-like function. Unless it
+doesn't have the exact same signature, an additional wrapper is
+required. Even if it has the same signature it might be a good
+idea to wrap it, so that `magit-prompt-with-default' can be used.
-(defun magit-builtin-completing-read
- (prompt choices &optional predicate require-match initial-input hist def)
- "Magit wrapper for standard `completing-read' function."
- (let ((reply (completing-read
- (if (and def (> (length prompt) 2)
- (string-equal ": " (substring prompt -2)))
- (format "%s (default %s): " (substring prompt 0 -2) def)
- prompt)
- choices predicate require-match initial-input hist def)))
+See `completing-read' for the meanings of the arguments, but note
+that this wrapper makes the following changes:
+
+- If REQUIRE-MATCH is nil and the user exits without a choise,
+ then return nil instead of an empty string.
+
+- If REQUIRE-MATCH is non-nil and the users exits without a
+ choise, then raise a user-error.
+
+- For historic reasons \": \" is appended to PROMPT. This will
+ likely be fixed.
+
+- If a `magit-completing-read-function' is used which in turn
+ uses `magit-prompt-with-completion' and DEF is non-nil, then
+ PROMPT is modified to end with \" (default DEF): \".
+
+The use of another completing function and/or wrapper obviously
+results in additional differences."
+ (let ((reply (funcall magit-completing-read-function
+ (concat prompt ": ") collection predicate
+ require-match initial-input hist def)))
(if (string= reply "")
(if require-match
(user-error "Nothing selected")
nil)
reply)))
+(defun magit-builtin-completing-read
+ (prompt choices &optional predicate require-match initial-input hist def)
+ "Magit wrapper for standard `completing-read' function."
+ (completing-read (magit-prompt-with-default prompt def)
+ choices predicate require-match
+ initial-input hist def))
+
(defun magit-ido-completing-read
(prompt choices &optional predicate require-match initial-input hist def)
- "ido-based completing-read almost-replacement."
+ "Ido-based completing-read almost-replacement."
(require 'ido)
(let ((reply (ido-completing-read
prompt
@@ -3777,7 +3797,7 @@ Read `completing-read' documentation for the meaning of the argument."
(defun magit-iswitchb-completing-read
(prompt choices &optional predicate require-match initial-input hist def)
- "iswitchb-based completing-read almost-replacement."
+ "Iswitchb-based completing-read almost-replacement."
(require 'iswitchb)
(let ((iswitchb-make-buflist-hook
(lambda ()
@@ -3786,6 +3806,12 @@ Read `completing-read' documentation for the meaning of the argument."
choices)))))
(iswitchb-read-buffer prompt (or initial-input def) require-match)))
+(defun magit-prompt-with-default (prompt def)
+ (if (and def (> (length prompt) 2)
+ (string-equal ": " (substring prompt -2)))
+ (format "%s (default %s): " (substring prompt 0 -2) def)
+ prompt))
+
;;;;; Revision Completion
(defvar magit-read-rev-history nil
@@ -3799,11 +3825,8 @@ Read `completing-read' documentation for the meaning of the argument."
elt)
(magit-list-interesting-refs uninteresting)))
(reply (magit-completing-read prompt interesting-refs nil nil nil
- 'magit-read-rev-history default))
- (rev (or (cdr (assoc reply interesting-refs)) reply)))
- (unless (or rev noselection)
- (user-error "No rev selected"))
- rev))
+ 'magit-read-rev-history default)))
+ (or (cdr (assoc reply interesting-refs)) reply)))
(defun magit-read-rev-with-default (prompt)
(magit-read-rev prompt (--when-let (or (magit-guess-branch) "HEAD")
@@ -3816,18 +3839,15 @@ Read `completing-read' documentation for the meaning of the argument."
'magit-read-rev-history))
(defun magit-read-remote-branch (prompt remote &optional default)
- (let ((branch (magit-completing-read
- prompt
- (cl-mapcan
- (lambda (b)
- (and (not (string-match " -> " b))
- (string-match (format "^ *%s/\\(.*\\)$"
- (regexp-quote remote)) b)
- (list (match-string 1 b))))
- (magit-git-lines "branch" "-r"))
- nil nil nil nil default)))
- (unless (string= branch "")
- branch)))
+ (magit-completing-read prompt
+ (cl-mapcan
+ (lambda (b)
+ (and (not (string-match " -> " b))
+ (string-match (format "^ *%s/\\(.*\\)$"
+ (regexp-quote remote)) b)
+ (list (match-string 1 b))))
+ (magit-git-lines "branch" "-r"))
+ nil nil nil nil default))
(defun magit-read-tag (prompt &optional require-match)
(magit-completing-read prompt (magit-git-lines "tag") nil
Something went wrong with that request. Please try again.