Subject: bug#41343: tab-bar-mode: Close tab on mouse-2 click

From: Juri Linkov <[email-masked]>
Date: Mon, 09 Aug 2021 10:03:28 +0300

forcemerge 41343 41342
tags 41342 + patch
tags 41343 + patch
thanks

> Are you talking about handle_tab_bar_click and the processing of
> TAB_BAR_EVENT in make_lispy_event?  Or are there other/additional
> parts to this processing?

So now I've completely redesigned the event processing on the tab bar.
With the following patch

'down-mouse-1' is bound to 'tab-bar-mouse-select-tab'
'down-mouse-2' is bound to 'tab-bar-mouse-close-tab'
'down-mouse-3' is bound to 'tab-bar-mouse-context-menu'

The context menu that pops up on down-mouse-3 was requested in bug#41342.
Now also it should be possible even to implement drag-drop to move tabs
with the mouse since they produce normal mouse events that now are accepted
by the tab bar.

All aforementioned bindings work on X, xterm, and even on console.
For example, clicking mouse-3 on the tab bar displays a menu
with blue background on a console terminal.

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 7459e1b78c..162dc43876 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -224,6 +224,11 @@ tab-bar-mode
       (tab-bar--define-keys)
     (tab-bar--undefine-keys)))
 
+(defun tab--symbol-to-number (symbol)
+  (unless (eq symbol 'current-tab)
+    (string-to-number
+     (string-replace "tab-" "" (format "%S" symbol)))))
+
 (defun tab-bar-handle-mouse (event)
   "Text-mode emulation of switching tabs on the tab bar.
 This command is used when you click the mouse in the tab bar
@@ -238,18 +243,52 @@ tab-bar-handle-mouse
                  (lambda (key binding)
                    (when (eq (car-safe binding) 'menu-item)
                      (when (> (+ column (length (nth 1 binding))) x-position)
-                       (if (get-text-property (- x-position column) 'close-tab (nth 1 binding))
-                           (let* ((close-key (vector (intern (format "C-%s" key))))
-                                  (close-def (lookup-key keymap close-key)))
-                             (when close-def
-                               (call-interactively close-def)))
-                         (call-interactively (nth 2 binding)))
+                       (if (get-text-property
+                            (- x-position column) 'close-tab (nth 1 binding))
+                           (tab-bar-close-tab (tab--symbol-to-number key))
+                         (if (nth 2 binding)
+                             (call-interactively (nth 2 binding))
+                           (tab-bar-select-tab (tab--symbol-to-number key))))
                        (throw 'done t))
                      (setq column (+ column (length (nth 1 binding))))))
                  keymap))
         ;; Clicking anywhere outside existing tabs will add a new tab
         (tab-bar-new-tab)))))
 
+(defun tab-bar-mouse-select-tab (event)
+  (interactive "e")
+  (if (posn-window (event-start event))
+      (let* ((tab (posn-string (event-start event)))
+             (tab-symbol (nth 0 tab))
+             (tab-number (tab--symbol-to-number tab-symbol)))
+        (if (nth 1 tab)
+            (tab-bar-close-tab tab-number)
+          (let ((binding (lookup-key (cons 'keymap (nreverse (current-active-maps)))
+                                     (vector 'tab-bar tab-symbol))))
+            (if binding
+                (call-interactively binding)
+              (tab-bar-select-tab tab-number)))))
+    ;; TTY
+    (tab-bar-handle-mouse event)))
+
+(defun tab-bar-mouse-close-tab (event)
+  (interactive "e")
+  (tab-bar-close-tab (tab--symbol-to-number
+                      (nth 0 (posn-string (event-start event))))))
+
+(defun tab-bar-mouse-context-menu (event)
+  (interactive "e")
+  (let* ((tab (posn-string (event-start event)))
+         (tab-number (tab--symbol-to-number (nth 0 tab)))
+         (menu (make-sparse-keymap "Context Menu")))
+
+    (define-key-after menu [close]
+      `(menu-item "Close" (lambda () (interactive)
+                            (tab-bar-close-tab ,tab-number))
+                  :help "Close the tab"))
+
+    (popup-menu menu event)))
+
 (defun toggle-tab-bar-mode-from-frame (&optional arg)
   "Toggle tab bar on or off, based on the status of the current frame.
 Used in the Show/Hide menu, to have the toggle reflect the current frame.
@@ -614,19 +654,12 @@ tab-bar--format-tab
      `((,(intern (format "tab-%i" i))
         menu-item
         ,(funcall tab-bar-tab-name-format-function tab i)
-        ,(or
-          (alist-get 'binding tab)
-          `(lambda ()
-             (interactive)
-             (tab-bar-select-tab ,i)))
+        ,(alist-get 'binding tab)
         :help "Click to visit tab"))))
-   `((,(if (eq (car tab) 'current-tab) 'C-current-tab (intern (format "C-tab-%i" i)))
-      menu-item ""
-      ,(or
-        (alist-get 'close-binding tab)
-        `(lambda ()
-           (interactive)
-           (tab-bar-close-tab ,i)))))))
+   (when (alist-get 'close-binding tab)
+     `((,(if (eq (car tab) 'current-tab) 'C-current-tab (intern (format "C-tab-%i" i)))
+        menu-item ""
+        ,(alist-get 'close-binding tab))))))
 
 (defun tab-bar-format-tabs ()
   (let ((i 0))
@@ -767,7 +800,11 @@ tab-bar-format-list
 (defun tab-bar-make-keymap-1 ()
   "Generate an actual keymap from `tab-bar-map', without caching."
   (append
-   '(keymap (mouse-1 . tab-bar-handle-mouse))
+   '(keymap (down-mouse-1 . tab-bar-mouse-select-tab)
+            (mouse-1 . ignore)
+            (down-mouse-2 . tab-bar-mouse-close-tab)
+            (mouse-2 . ignore)
+            (down-mouse-3 . tab-bar-mouse-context-menu))
    (tab-bar-format-list tab-bar-format)))
 
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 33fcaa4c07..f4c7575b35 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3415,8 +3415,8 @@ #define TTY_CAP_STRIKE_THROUGH	0x20
                                         NativeRectangle *nr);
 extern Lisp_Object find_hot_spot (Lisp_Object, int, int);
 
-extern void handle_tab_bar_click (struct frame *,
-                                   int, int, bool, int);
+extern Lisp_Object handle_tab_bar_click (struct frame *,
+					 int, int, bool, int);
 extern void handle_tool_bar_click (struct frame *,
                                    int, int, bool, int);
 
diff --git a/src/keyboard.c b/src/keyboard.c
index 820229cf8f..5dabad98a8 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -5647,6 +5647,12 @@ make_lispy_event (struct input_event *event)
 
 	    position = make_lispy_position (f, event->x, event->y,
 					    event->timestamp);
+
+	    if (CONSP (event->arg) && EQ (XCAR (event->arg), Qtab_bar))
+	      {
+		XSETCAR (XCDR (position), Qtab_bar);
+		position = nconc2(position, Fcons (XCDR (event->arg), Qnil));
+	      }
 	  }
 #ifndef USE_TOOLKIT_SCROLL_BARS
 	else
diff --git a/src/term.c b/src/term.c
index c995a4499c..89b3568003 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2568,21 +2568,8 @@ handle_one_term_event (struct tty_display_info *tty, Gpm_Event *event)
     {
       f->mouse_moved = 0;
       term_mouse_click (&ie, event, f);
-      /* eassert (ie.kind == MOUSE_CLICK_EVENT); */
-      if (tty_handle_tab_bar_click (f, event->x, event->y,
-                                    (ie.modifiers & down_modifier) != 0, &ie))
-        {
-          /* eassert (ie.kind == MOUSE_CLICK_EVENT
-           *          || ie.kind == TAB_BAR_EVENT); */
-          /* tty_handle_tab_bar_click stores 2 events in the event
-             queue, so we are done here.  */
-          /* FIXME: Actually, `tty_handle_tab_bar_click` returns true
-             without storing any events, when
-             (ie.modifiers & down_modifier) != 0  */
-          count += 2;
-          return count;
-        }
-      /* eassert (ie.kind == MOUSE_CLICK_EVENT); */
+      ie.arg = tty_handle_tab_bar_click (f, event->x, event->y,
+					 (ie.modifiers & down_modifier) != 0, &ie);
       kbd_buffer_store_event (&ie);
       count++;
     }
diff --git a/src/termchar.h b/src/termchar.h
index f50c1bfb6e..7ab9337fbe 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -234,7 +234,7 @@ #define FRAME_TTY(f)                            \
 #define CURTTY() FRAME_TTY (SELECTED_FRAME())
 
 struct input_event;
-extern bool tty_handle_tab_bar_click (struct frame *, int, int, bool,
-				      struct input_event *);
+extern Lisp_Object tty_handle_tab_bar_click (struct frame *, int, int, bool,
+					     struct input_event *);
 
 #endif /* EMACS_TERMCHAR_H */
diff --git a/src/w32inevt.c b/src/w32inevt.c
index 1255072b7f..9a69b32bcb 100644
--- a/src/w32inevt.c
+++ b/src/w32inevt.c
@@ -586,9 +586,8 @@ do_mouse_event (MOUSE_EVENT_RECORD *event,
       int x = event->dwMousePosition.X;
       int y = event->dwMousePosition.Y;
       struct frame *f = get_frame ();
-      if (tty_handle_tab_bar_click (f, x, y, (button_state & mask) != 0,
-				    emacs_ev))
-	return 0;	/* tty_handle_tab_bar_click adds the event to queue */
+      emacs_ev->arg = tty_handle_tab_bar_click (f, x, y, (button_state & mask) != 0,
+						emacs_ev);
 
       emacs_ev->modifiers |= ((button_state & mask)
 			      ? down_modifier : up_modifier);
@@ -597,7 +596,6 @@ do_mouse_event (MOUSE_EVENT_RECORD *event,
       XSETFASTINT (emacs_ev->x, x);
       XSETFASTINT (emacs_ev->y, y);
       XSETFRAME (emacs_ev->frame_or_window, f);
-      emacs_ev->arg = Qnil;
 
       return 1;
     }
diff --git a/src/w32term.c b/src/w32term.c
index ad4d1a3282..c9570b0c67 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -168,8 +168,8 @@ #define SM_CYVIRTUALSCREEN 79
 int w32_message_fd = -1;
 #endif /* CYGWIN */
 
-static void w32_handle_tab_bar_click (struct frame *,
-                                      struct input_event *);
+static Lisp_Object w32_handle_tab_bar_click (struct frame *,
+					     struct input_event *);
 static void w32_handle_tool_bar_click (struct frame *,
                                        struct input_event *);
 static void w32_define_cursor (Window, Emacs_Cursor);
@@ -3684,17 +3684,17 @@ w32_mouse_position (struct frame **fp, int insist, Lisp_Object *bar_window,
    frame-relative coordinates X/Y.  EVENT_TYPE is either ButtonPress
    or ButtonRelease.  */
 
-static void
+static Lisp_Object
 w32_handle_tab_bar_click (struct frame *f, struct input_event *button_event)
 {
   int x = XFIXNAT (button_event->x);
   int y = XFIXNAT (button_event->y);
 
   if (button_event->modifiers & down_modifier)
-    handle_tab_bar_click (f, x, y, 1, 0);
+    return handle_tab_bar_click (f, x, y, 1, 0);
   else
-    handle_tab_bar_click (f, x, y, 0,
-                          button_event->modifiers & ~up_modifier);
+    return handle_tab_bar_click (f, x, y, 0,
+				 button_event->modifiers & ~up_modifier);
 }

@@ -5186,6 +5186,7 @@ w32_read_socket (struct terminal *terminal,
 	  {
             /* If we decide we want to generate an event to be seen
                by the rest of Emacs, we put it here.  */
+	    Lisp_Object tab_bar_key = Qnil;
 	    bool tab_bar_p = 0;
 	    bool tool_bar_p = 0;
 	    int button = 0;
@@ -5208,12 +5209,12 @@ w32_read_socket (struct terminal *terminal,
 
                     if (EQ (window, f->tab_bar_window))
                       {
-                        w32_handle_tab_bar_click (f, &inev);
+                        tab_bar_key = w32_handle_tab_bar_click (f, &inev);
                         tab_bar_p = 1;
                       }
                   }
 
-                if (tab_bar_p
+                if ((tab_bar_p && NILP (tab_bar_key))
 		    || (dpyinfo->w32_focus_frame
 			&& f != dpyinfo->w32_focus_frame
 			/* This does not help when the click happens in
@@ -5221,6 +5222,9 @@ w32_read_socket (struct terminal *terminal,
 			&& !frame_ancestor_p (f, dpyinfo->w32_focus_frame)))
 		  inev.kind = NO_EVENT;
 
+		  if (!NILP (tab_bar_key))
+		    inev.arg = tab_bar_key;
+
                 /* Is this in the tool-bar?  */
                 if (WINDOWP (f->tool_bar_window)
                     && WINDOW_TOTAL_LINES (XWINDOW (f->tool_bar_window)))
diff --git a/src/xdisp.c b/src/xdisp.c
index e62f7e3df6..0cbfa4bfff 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13745,7 +13745,7 @@ get_tab_bar_item (struct frame *f, int x, int y, struct glyph **glyph,
    false for button release.  MODIFIERS is event modifiers for button
    release.  */
 
-void
+Lisp_Object
 handle_tab_bar_click (struct frame *f, int x, int y, bool down_p,
 		      int modifiers)
 {
@@ -13763,12 +13763,12 @@ handle_tab_bar_click (struct frame *f, int x, int y, bool down_p,
       /* If the button is released on a tab other than the one where
 	 it was pressed, don't generate the tab-bar button click event.  */
       || (ts != 0 && !down_p))
-    return;
+    return Qnil;
 
   /* If item is disabled, do nothing.  */
   enabled_p = AREF (f->tab_bar_items, prop_idx + TAB_BAR_ITEM_ENABLED_P);
   if (NILP (enabled_p))
-    return;
+    return Qnil;
 
   if (down_p)
     {
@@ -13779,24 +13779,15 @@ handle_tab_bar_click (struct frame *f, int x, int y, bool down_p,
     }
   else
     {
-      Lisp_Object key, frame;
-      struct input_event event;
-      EVENT_INIT (event);
-
       /* Show item in released state.  */
       if (!NILP (Vmouse_highlight))
 	show_mouse_face (hlinfo, DRAW_IMAGE_RAISED);
-
-      key = AREF (f->tab_bar_items, prop_idx + TAB_BAR_ITEM_KEY);
-
-      XSETFRAME (frame, f);
-      event.kind = TAB_BAR_EVENT;
-      event.frame_or_window = frame;
-      event.arg = key;
-      event.modifiers = close_p ? ctrl_modifier | modifiers : modifiers;
-      kbd_buffer_store_event (&event);
       f->last_tab_bar_item = -1;
     }
+
+  return list3 (Qtab_bar,
+		AREF (f->tab_bar_items, prop_idx + TAB_BAR_ITEM_KEY),
+		close_p ? Qt : Qnil);
 }

@@ -13920,14 +13911,14 @@ tty_get_tab_bar_item (struct frame *f, int x, int *idx, ptrdiff_t *end)
    structure, store it in keyboard queue, and return true; otherwise
    return false.  MODIFIERS are event modifiers for generating the tab
    release event.  */
-bool
+Lisp_Object
 tty_handle_tab_bar_click (struct frame *f, int x, int y, bool down_p,
 			  struct input_event *event)
 {
   /* Did they click on the tab bar?  */
   if (y < FRAME_MENU_BAR_LINES (f)
       || y >= FRAME_MENU_BAR_LINES (f) + FRAME_TAB_BAR_LINES (f))
-    return false;
+    return Qnil;
 
   /* Find the tab-bar item where the X,Y coordinates belong.  */
   int prop_idx;
@@ -13935,46 +13926,33 @@ tty_handle_tab_bar_click (struct frame *f, int x, int y, bool down_p,
   Lisp_Object caption = tty_get_tab_bar_item (f, x, &prop_idx, &clen);
 
   if (NILP (caption))
-    return false;
+    return Qnil;
 
   if (NILP (AREF (f->tab_bar_items,
 		  prop_idx * TAB_BAR_ITEM_NSLOTS + TAB_BAR_ITEM_ENABLED_P)))
-    return false;
+    return Qnil;
 
   if (down_p)
     f->last_tab_bar_item = prop_idx;
   else
     {
-      /* Force reset of up_modifier bit from the event modifiers.  */
-      if (event->modifiers & up_modifier)
-        event->modifiers &= ~up_modifier;
-
-      /* Generate a TAB_BAR_EVENT event.  */
-      Lisp_Object frame;
-      Lisp_Object key = AREF (f->tab_bar_items,
-			      prop_idx * TAB_BAR_ITEM_NSLOTS
-			      + TAB_BAR_ITEM_KEY);
-      /* Kludge alert: we assume the last two characters of a tab
-	 label are " x", and treat clicks on those 2 characters as a
-	 Close Tab command.  */
-      eassert (STRINGP (caption));
-      int lastc = SSDATA (caption)[SCHARS (caption) - 1];
-      bool close_p = false;
-      if ((x == clen - 1 || (clen > 1 && x == clen - 2)) && lastc == 'x')
-	close_p = true;
-
-      event->code = 0;
-      XSETFRAME (frame, f);
-      event->kind = TAB_BAR_EVENT;
-      event->frame_or_window = frame;
-      event->arg = key;
-      if (close_p)
-	event->modifiers |= ctrl_modifier;
-      kbd_buffer_store_event (event);
       f->last_tab_bar_item = -1;
     }
 
-  return true;
+  /* Generate a TAB_BAR_EVENT event.  */
+  Lisp_Object key = AREF (f->tab_bar_items,
+			  prop_idx * TAB_BAR_ITEM_NSLOTS
+			  + TAB_BAR_ITEM_KEY);
+  /* Kludge alert: we assume the last two characters of a tab
+     label are " x", and treat clicks on those 2 characters as a
+     Close Tab command.  */
+  eassert (STRINGP (caption));
+  int lastc = SSDATA (caption)[SCHARS (caption) - 1];
+  bool close_p = false;
+  if ((x == clen - 1 || (clen > 1 && x == clen - 2)) && lastc == 'x')
+    close_p = true;
+
+  return list3 (Qtab_bar, key, close_p ? Qt : Qnil);
 }
 
 
diff --git a/src/xterm.c b/src/xterm.c
index 1887c3255d..80fa747b97 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -9166,6 +9166,7 @@ handle_one_xevent (struct x_display_info *dpyinfo,
       {
         /* If we decide we want to generate an event to be seen
            by the rest of Emacs, we put it here.  */
+        Lisp_Object tab_bar_key = Qnil;
         bool tab_bar_p = false;
         bool tool_bar_p = false;
 
@@ -9215,7 +9216,7 @@ handle_one_xevent (struct x_display_info *dpyinfo,
                 tab_bar_p = EQ (window, f->tab_bar_window);
 
                 if (tab_bar_p && event->xbutton.button < 4)
-		  handle_tab_bar_click
+		  tab_bar_key = handle_tab_bar_click
 		    (f, x, y, event->xbutton.type == ButtonPress,
 		     x_x_to_emacs_modifiers (dpyinfo, event->xbutton.state));
               }
@@ -9239,7 +9240,7 @@ handle_one_xevent (struct x_display_info *dpyinfo,
               }
 #endif /* !USE_GTK */
 
-            if (!tab_bar_p && !tool_bar_p)
+            if (!(tab_bar_p && NILP (tab_bar_key)) && !tool_bar_p)
 #if defined (USE_X_TOOLKIT) || defined (USE_GTK)
               if (! popup_activated ())
 #endif
@@ -9257,6 +9258,9 @@ handle_one_xevent (struct x_display_info *dpyinfo,
                     }
                   else
                     x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+
+		  if (!NILP (tab_bar_key))
+		    inev.ie.arg = tab_bar_key;
                 }
             if (FRAME_X_EMBEDDED_P (f))
               xembed_send_message (f, event->xbutton.time,