bug#13692: mouse clicks in vc-dir buffers accidentally changing marks

[Dmitry Gutov <dgutov@yandex.ru>]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

On 23.01.2021 21:02, Lars Ingebrigtsen wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> We're talking about slightly different things.
>>
>> When I hear "mouse-2 binding", I think the global bidning in
>> vc-dir-mode-map, which affects how mouse-2 works also when clicking
>> outside of any buttons (on whitespace beside the buttons). And I think
>> that behavior is relatively weird and could be removed.
> 
> I didn't even notice that `mouse-2' did stuff when outside the
> "buttons".  And what it does is quite odd...  hm, no, if you click after
> the final line, it'll mark the last line, and before the first, it'll
> mark the first, so perhaps that's logical?

I meant more like between the buttons, but on the same line as them. The 
"before" and "after" behaviors are logical, but the whole binding is 
very non-discoverable. Nor does it work on file names.

>> But we also have the behavior of mouse-1 and mouse-2 clicks on the
>> "status" button. Which seem fairly sensible to me, but might be the
>> cause of Glenn's annoyance in this report because it's relatively easy
>> to click mouse-1 anywhere in the buffer by mistake.
> 
> Yes, I think he was complaining about the "status" button.
> 
>> Right. But if that is what is at issue here, I think the question
>> becomes whether to remove the "buttons" from that column (and keep the
>> mouse-2 binding globally) or whether to remove the mouse-2 binding,
>> but set up the buttons to continue working. Or make mouse-1 stop
>> working on the buttons but keep mouse-2 working everywhere (except the
>> file names, I guess?).
> 
> I think removing the global mouse bindings and only having them on the
> "buttons" would be least surprising -- that's how most modes work.

The attached patch seems to do that, except for the "status" buttons 
beside directory names, which seem to be printed by ewoc in some other 
way, and thus don't go through vc-dir-printer. Which is a new different 
kind of awkward.

We're also solving a different problem than the one which was reported.

So I'm not sure about the change, TBH.

[-- Attachment #2: vc-dir-no-mouse-2.diff --]
[-- Type: text/x-patch, Size: 2216 bytes --]

diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index bbb73240be..9d0808c043 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -300,7 +300,6 @@ vc-dir-mode-map
     (define-key map "\C-o" 'vc-dir-display-file)
     (define-key map "\C-c\C-c" 'vc-dir-kill-dir-status-process)
     (define-key map [down-mouse-3] 'vc-dir-menu)
-    (define-key map [mouse-2] 'vc-dir-toggle-mark)
     (define-key map [follow-link] 'mouse-face)
     (define-key map "x" 'vc-dir-hide-up-to-date)
     (define-key map [?\C-k] 'vc-dir-kill-line)
@@ -1085,7 +1084,6 @@ vc-dir-mode
       as the current file
   - if the cursor is on a directory: unmark all child files
   - with a prefix argument: unmark all files
-mouse-2  - toggles the mark state
 
 VC commands
 VC commands in the `C-x v' prefix can be used.
@@ -1392,6 +1390,12 @@ vc-default-dir-extra-headers
    (propertize "Please add backend specific headers here.  It's easy!"
 	       'face 'font-lock-warning-face)))
 
+(defvar vc-dir-status-mouse-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mouse-2] 'vc-dir-toggle-mark)
+    map)
+  "Local keymap for toggling mark.")
+
 (defvar vc-dir-filename-mouse-map
    (let ((map (make-sparse-keymap)))
      (define-key map [mouse-2] 'vc-dir-find-file-other-window)
@@ -1418,7 +1422,8 @@ vc-default-dir-printer
 		  ((memq state '(missing conflict)) 'font-lock-warning-face)
 		  ((eq state 'edited) 'font-lock-constant-face)
 		  (t 'font-lock-variable-name-face))
-      'mouse-face 'highlight)
+      'mouse-face 'highlight
+      'keymap vc-dir-status-mouse-map)
      " "
      (propertize
       (format "%s" filename)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index a9ee28e3aa..94fac3a83b 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -481,7 +481,8 @@ vc-git-dir-printer
       'face (cond ((eq state 'up-to-date) 'font-lock-builtin-face)
 		  ((eq state 'missing) 'font-lock-warning-face)
 		  (t 'font-lock-variable-name-face))
-      'mouse-face 'highlight)
+      'mouse-face 'highlight
+      'keymap vc-dir-status-mouse-map)
      "  " (vc-git-permissions-as-string old-perm new-perm)
      "    "
      (propertize (vc-git-escape-file-name (vc-dir-fileinfo->name info))