Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix incorrect cursor location in magit-key-mode #514

Merged
merged 1 commit into from

2 participants

@chumpage

After setting an argument value, point would be in the wrong spot. Fix
this by saving which option point is at before redrawing, then jumping
to that option after redrawing. Add a new function
magit-key-mode-build-exec-point-alist to help with this.

@chumpage chumpage Fix incorrect cursor location in magit-key-mode
After setting an argument value, point would be in the wrong spot. Fix
this by saving which option point is at before redrawing, then jumping
to that option after redrawing. Add a new function
magit-key-mode-build-exec-point-alist to help with this.
92fdb63
@chumpage

Easiest way to repro the problem:

  1. Open magit on a repo.
  2. 'l' to bring up the log menu.
  3. Notice point is at the 'l', the first item in the actions section.
  4. '=g' and enter something for the grep value (e.g. "hello").
  5. Notice point is no longer at the 'l' in the actions section.
@sigma
Owner

I do agree there's a problem there, but I'm not sure this fix goes into the right direction.
Actually all this derives from the fact that we're erasing the entire buffer and rebuilding it each time anything changes.
I think that's madness and that we should fix this instead.

Like, we could store a list of overlays for each option, and have them populated/altered whenever something changes.
That way the cursor would stay in place by itself, simply because it would never have to move !

@chumpage

I agree it'd be better to not redraw the buffer every time. Until that's done though, why not take my change? It at least fixes the bug with the cursor position. Once magit-key-mode is changed to not redraw the buffer every time, my code to fix the cursor pos would be obsolete and could be easily deleted.

@sigma sigma merged commit 92fdb63 into magit:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 29, 2012
  1. @chumpage

    Fix incorrect cursor location in magit-key-mode

    chumpage authored
    After setting an argument value, point would be in the wrong spot. Fix
    this by saving which option point is at before redrawing, then jumping
    to that option after redrawing. Add a new function
    magit-key-mode-build-exec-point-alist to help with this.
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 1 deletion.
  1. +20 −1 magit-key-mode.el
View
21 magit-key-mode.el
@@ -251,6 +251,18 @@ FOR-GROUP."
(error "Nothing at point to do.")))
(def (lookup-key (current-local-map) key)))
(call-interactively def)))
+
+(defun magit-key-mode-build-exec-point-alist ()
+ (save-excursion
+ (goto-char (point-min))
+ (let* ((exec (get-text-property (point) 'key-group-executor))
+ (exec-alist (if exec `((,exec . ,(point))) nil)))
+ (do nil ((eobp) (nreverse exec-alist))
+ (when (not (eq exec (get-text-property (point) 'key-group-executor)))
+ (setq exec (get-text-property (point) 'key-group-executor))
+ (when exec (push (cons exec (point)) exec-alist)))
+ (forward-char)))))
+
(defun magit-key-mode-jump-to-next-exec ()
"Jump to the next action/args/option point."
(interactive)
@@ -396,6 +408,8 @@ highlighted before the description."
(defun magit-key-mode-redraw (for-group)
"(re)draw the magit key buffer."
(let ((buffer-read-only nil)
+ (current-exec (get-text-property (point) 'key-group-executor))
+ (new-exec-pos)
(old-point (point))
(is-first (zerop (buffer-size)))
(actions-p nil))
@@ -405,10 +419,15 @@ highlighted before the description."
(setq actions-p (magit-key-mode-draw for-group))
(delete-trailing-whitespace)
(setq mode-name "magit-key-mode" major-mode 'magit-key-mode)
+ (when current-exec
+ (setq new-exec-pos (cdr (assoc current-exec (magit-key-mode-build-exec-point-alist)))))
(if (and is-first actions-p)
(progn (goto-char actions-p)
(magit-key-mode-jump-to-next-exec))
- (goto-char old-point)))
+ (if new-exec-pos
+ (progn (goto-char new-exec-pos)
+ (skip-chars-forward " "))
+ (goto-char old-point))))
(setq buffer-read-only t)
(fit-window-to-buffer))
Something went wrong with that request. Please try again.