magit-rebase--todo: Prevent false-positives

Previously, if `git-rebase-show-instructions' was non-nil, it was
possible that a line containing usage information was mistaken for
a rebase action.

Alternatively we could use `with-editor-pre-finish-hook' to flush the
usage information after the user has seen them, just like we flush them
earlier in `git-rebase-mode', iff `git-rebase-show-instructions' is nil.

3 files changed, 14 insertions, 9 deletions

diff --git a/lisp/git-rebase.el b/lisp/git-rebase.el
index 64a46d3..e033b2b 100644
--- a/lisp/git-rebase.el
+++ b/lisp/git-rebase.el
@@ -346,14 +346,18 @@ region is active, act on all lines touched by the region."
                       " ?\\(?4:.*\\)"))))
 
 ;;;###autoload
-(defun git-rebase-current-line ()
+(defun git-rebase-current-line (&optional batch)
   "Parse current line into a `git-rebase-action' instance.
 If the current line isn't recognized as a rebase line, an
-instance with all nil values is returned."
+instance with all nil values is returned, unless optional
+BATCH is non-nil, in which case nil is returned.  Non-nil
+BATCH also ignores commented lines."
   (save-excursion
     (goto-char (line-beginning-position))
-    (if-let ((re-start (concat "^\\(?5:" (regexp-quote comment-start)
-                               "\\)? *"))
+    (if-let ((re-start (if batch
+                           "^"
+                         (format "^\\(?5:%s\\)? *"
+                                 (regexp-quote comment-start))))
              (type (seq-some (pcase-lambda (`(,type . ,re))
                                (let ((case-fold-search nil))
                                  (and (looking-at (concat re-start re)) type)))
@@ -367,8 +371,9 @@ instance with all nil values is returned."
          :target         (match-string-no-properties 3)
          :trailer        (match-string-no-properties 4)
          :comment-p      (and (match-string 5) t))
-      ;; Use empty object rather than nil to ease handling.
-      (git-rebase-action))))
+      (and (not batch)
+           ;; Use empty object rather than nil to ease handling.
+           (git-rebase-action)))))
 
 (defun git-rebase-set-action (action)
   "Set action of commit line to ACTION.
diff --git a/lisp/magit-diff.el b/lisp/magit-diff.el
index 1548e8b..784b743 100644
--- a/lisp/magit-diff.el
+++ b/lisp/magit-diff.el
@@ -50,7 +50,7 @@
 (declare-function magit-blame-mode "magit-blame" (&optional arg))
 (defvar magit-blame-mode)
 ;; For `magit-diff-show-or-scroll'
-(declare-function git-rebase-current-line "git-rebase" ())
+(declare-function git-rebase-current-line "git-rebase" (&optional batch))
 ;; For `magit-diff-unmerged'
 (declare-function magit-merge-in-progress-p "magit-merge" ())
 (declare-function magit--merge-range "magit-merge" (&optional head))
diff --git a/lisp/magit-sequence.el b/lisp/magit-sequence.el
index 90e352a..757bd59 100644
--- a/lisp/magit-sequence.el
+++ b/lisp/magit-sequence.el
@@ -31,7 +31,7 @@
 (require 'magit)
 
 ;; For `magit-rebase--todo'.
-(declare-function git-rebase-current-line "git-rebase" ())
+(declare-function git-rebase-current-line "git-rebase" (&optional batch))
 (eval-when-compile
   (cl-pushnew 'action-type eieio--known-slot-names)
   (cl-pushnew 'action eieio--known-slot-names)
@@ -997,7 +997,7 @@ status buffer (i.e., the reverse of how they will be applied)."
       (insert-file-contents
        (expand-file-name "rebase-merge/git-rebase-todo" (magit-gitdir)))
       (while (not (eobp))
-        (when-let ((obj (git-rebase-current-line)))
+        (when-let ((obj (git-rebase-current-line t)))
           (push obj actions)
           (when (memq (oref obj action-type) '(commit merge))
             (push obj commits)))