patch 9.1.1927: Wayland: clipboard code too complex

Problem:  Wayland: clipboard code too complex
Solution: Simplify clipboard related code around W23/W24
          (Foxe Chen).

Improve Wayland and clipboard related code:

- improve documentation
- remove unused code
- fix error handling
- remove warning per Clipboard_T

closes: #18794

Signed-off-by: Foxe Chen <[email protected]>
Signed-off-by: Christian Brabandt <[email protected]>

Author: 64-bitman

runtime/doc/options.txt

@@ -1,4 +1,4 @@
-*options.txt*	For Vim version 9.1.  Last change: 2025 Nov 23
+*options.txt*	For Vim version 9.1.  Last change: 2025 Nov 25
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -1843,9 +1843,9 @@ A jump table for the options with a short description can be found at |Q_op|.
 			option, yank operations (but not delete, change or
 			put) will additionally copy the text into register
 			'*'.  If Wayland is being used and the compositor does
-			not support the primary-selection-unstable-v1
-			protocol, then the regular selection is used in its
-			place.  Only available with the |+X11| or
+			not support the primary selection then the regular
+			selection is used in its place.  Only available with
+			the |+X11| or
 			|+wayland_clipboard| feature.  Availability can be
 			checked with: >
 			if has('unnamedplus')

runtime/doc/wayland.txt

@@ -1,4 +1,4 @@
-*wayland.txt*	For Vim version 9.1.  Last change: 2025 Nov 09
+*wayland.txt*	For Vim version 9.1.  Last change: 2025 Nov 25
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -16,7 +16,8 @@ Wayland Protocol Support				*wayland*
 Functionality such as the clipboard for Wayland requires a seat to use.  A
 Wayland seat can consist of a keyboard, pointer, and touch device(s).  The
 seat to use can be set with the 'wlseat' option.  Only useful if you use
-multiple Wayland seats in the same Wayland session.
+multiple Wayland seats in the same Wayland session.  If you don't know what
+this is means, then it likely does not matter for you.
 
 							*wayland-gui*
 See |gui-wayland|.  Please note that when using the GUI, Vim uses the toolkit
@@ -63,11 +64,18 @@ should be bundled with libwayland on your system: >
 If you don't get any match, then please see |wayland-focus-steal| for more
 information.
 
+Some compositors that are known to support either or both protocols:
+    1. KWin (KDE)
+    2. wlroots based (Sway, Labwc)
+    3. Niri
+    4. Hyprland
+    5. Wayfire
+
 If you come from X11, then the regular Wayland selection is equivalent to the
 CLIPBOARD selection in X11, and the primary Wayland selection equates to the
-X11 primary selection.  Accessing these selections is the same as X11 in Vim,
+X11 PRIMARY selection.  Accessing these selections is the same as X11 in Vim,
 in which the + register is the regular selection, and the * register is the
-primary selection, note that your compositor may not support primary
+primary selection.  Note that your compositor may not support primary
 selections, see |wayland-primary-selection| for more details.
 
 							*wayland-persist*

src/clipboard.c

@@ -100,10 +100,6 @@ typedef struct {
 
     clip_wl_selection_T regular;
     clip_wl_selection_T primary;
-
-    // Array of file descriptors of clients we are sending data to. These should
-    // be polled for POLLOUT and have the respective callback called for each.
-    garray_T write_fds;
 } clip_wl_T;
 
 // Mime types we support sending and receiving
@@ -172,22 +168,6 @@ clip_init(int can_use)
     }
 }
 
-    static void
-clip_init_single(Clipboard_T *cb, int can_use)
-{
-    // No need to init again if cbd is already available
-    if (can_use && cb->available)
-	return;
-
-    cb->available  = can_use;
-    cb->owned      = FALSE;
-    cb->start.lnum = 0;
-    cb->start.col  = 0;
-    cb->end.lnum   = 0;
-    cb->end.col    = 0;
-    cb->state      = SELECT_CLEARED;
-}
-
 /*
  * Check whether the VIsual area has changed, and if so try to become the owner
  * of the selection, and free any old converted selection we may still have
@@ -2428,7 +2408,7 @@ adjust_clip_reg(int *rp)
     if ((!clip_star.available && *rp == '*') ||
 	   (!clip_plus.available && *rp == '+'))
     {
-	msg_warn_missing_clipboard(!clip_plus.available, !clip_star.available);
+	msg_warn_missing_clipboard();
 	*rp = 0;
     }
 }
@@ -2874,6 +2854,8 @@ clip_init_wayland(void)
     if (wayland_ct == NULL)
 	return FAIL;
 
+    memset(&clip_wl, 0, sizeof(clip_wl));
+
     clip_wl.seat = vwl_connection_get_seat(wayland_ct, (char *)p_wse);
 
     if (clip_wl.seat == NULL)
@@ -2891,12 +2873,14 @@ clip_init_wayland(void)
 	    clip_wl.regular.available = true;
 	else
 	{
+	    // Shouldn't happen
 	    vwl_data_device_manager_discard(clip_wl.regular.manager);
 	    clip_wl.regular.manager = NULL;
+	    return FAIL;
 	}
     }
 
-    // If we still don't support the primary selection, find one for it
+    // If we still don't support the primary selection, try finding one for it
     // specifically.
     if (!(supported & WAYLAND_SELECTION_PRIMARY))
     {
@@ -2917,10 +2901,12 @@ clip_init_wayland(void)
 	    }
 	}
     }
-    else if (clip_wl.regular.available)
+
+    if (clip_wl.regular.available && !clip_wl.primary.available)
     {
 	// The protocol supports both regular and primary selections, just use
-	// one data device manager and one data device.
+	// one data device manager and one data device. Or the primary selection
+	// is not supported, make it point to the regular selection instead.
 	clip_wl.primary.available = true;
 	clip_wl.primary.manager = clip_wl.regular.manager;
 	clip_wl.primary.device = clip_wl.regular.device;
@@ -3435,7 +3421,7 @@ clip_wl_owner_exists(Clipboard_T *cbd)
  * depending on the order of values in str.
  */
     static clipmethod_T
-get_clipmethod(char_u *str, bool *plus UNUSED, bool *star UNUSED)
+get_clipmethod(char_u *str)
 {
     int		len	= (int)STRLEN(str) + 1;
     char_u	*buf	= alloc(len);
@@ -3460,11 +3446,7 @@ get_clipmethod(char_u *str, bool *plus UNUSED, bool *star UNUSED)
 	    {
 #ifdef FEAT_WAYLAND_CLIPBOARD
 		if (clip_wl.regular.available || clip_wl.primary.available)
-		{
 		    method = CLIPMETHOD_WAYLAND;
-		    *plus = clip_wl.regular.available;
-		    *star = clip_wl.primary.available;
-		}
 #endif
 	    }
 	}
@@ -3485,7 +3467,6 @@ get_clipmethod(char_u *str, bool *plus UNUSED, bool *star UNUSED)
 		    // xterm_dpy will be set to NULL.
 		    xterm_update();
 		    method = CLIPMETHOD_X11;
-		    *plus = *star = TRUE;
 		}
 #endif
 	    }
@@ -3534,8 +3515,7 @@ clipmethod_to_str(clipmethod_T method)
     char *
 choose_clipmethod(void)
 {
-    bool regular = false, primary = false;
-    clipmethod_T method = get_clipmethod(p_cpm, &regular, &primary);
+    clipmethod_T method = get_clipmethod(p_cpm);
 
     if (method == CLIPMETHOD_FAIL)
 	return e_invalid_argument;
@@ -3569,10 +3549,8 @@ choose_clipmethod(void)
     // If we have a clipmethod that works now, then initialize clipboard
     else if (clipmethod == CLIPMETHOD_NONE && method != CLIPMETHOD_NONE)
     {
-	clip_init_single(&clip_plus, regular);
-	clip_init_single(&clip_star, primary);
-	clip_plus.did_warn = false;
-	clip_star.did_warn = false;
+	    clip_init(TRUE);
+	    did_warn_clipboard = false;
     }
     // Disown clipboard if we are switching to a new method
     else if (clipmethod != CLIPMETHOD_NONE && method != clipmethod)
@@ -3590,8 +3568,8 @@ choose_clipmethod(void)
 	if (!gui.in_use)
 #endif
 	{
-	    clip_init_single(&clip_plus, regular);
-	    clip_init_single(&clip_star, primary);
+	    clip_init(TRUE);
+	    did_warn_clipboard = false;
 	}
     }
 

src/globals.h

@@ -2067,6 +2067,9 @@ EXTERN char_u showcmd_buf[SHOWCMD_BUFLEN];
 EXTERN int	p_tgc_set INIT(= FALSE);
 #endif
 
+// If we've already warned about missing/unavailable clipboard
+EXTERN bool did_warn_clipboard INIT(= FALSE);
+
 #ifdef FEAT_CLIPBOARD
 EXTERN clipmethod_T clipmethod INIT(= CLIPMETHOD_NONE);
 #endif
@@ -2079,6 +2082,9 @@ EXTERN char *wayland_display_name INIT(= NULL);
 // Special mime type used to identify selection events that came from us setting
 // the selection. Is in format of "application/x-vim-instance-<pid>" where <pid>
 // is the PID of the Vim process. Set in main.c
+//
+// This is more reliable than just checking if our data source is non-NULL, as
+// that may be subject to data races in the Wayland protocol.
 EXTERN char wayland_vim_special_mime[
     sizeof("application/x-vim-instance-") + NUMBUFLEN - 1]; // Includes NUL
 

src/message.c

@@ -4177,37 +4177,17 @@ msg_advance(int col)
  * Warn about missing Clipboard Support
  */
     void
-msg_warn_missing_clipboard(bool plus UNUSED, bool star UNUSED)
+msg_warn_missing_clipboard(void)
 {
-#ifndef FEAT_CLIPBOARD
-    static bool did_warn;
-
-    if (!global_busy && !did_warn)
+    if (!global_busy && !did_warn_clipboard)
     {
-	msg(_("W24: Clipboard register not available. See :h W24"));
-	did_warn = true;
-    }
+#ifdef FEAT_CLIPBOARD
+	msg(_("W23: Clipboard register not available, using register 0"));
 #else
-    if (!global_busy)
-    {
-	if (plus && star && !clip_plus.did_warn && !clip_star.did_warn)
-	{
-	    msg(_("W23: Clipboard register not available, using register 0"));
-	    clip_plus.did_warn = true;
-	    clip_star.did_warn = true;
-	}
-	else if (plus && !clip_plus.did_warn)
-	{
-	    msg(_("W23: Clipboard register + not available, using register 0"));
-	    clip_plus.did_warn = true;
-	}
-	else if (star && !clip_star.did_warn)
-	{
-	    msg(_("W23: Clipboard register * not available, using register 0"));
-	    clip_star.did_warn = true;
-	}
-    }
+	msg(_("W24: Clipboard register not available. See :h W24"));
 #endif
+	did_warn_clipboard = TRUE;
+    }
 }
 
 #if defined(FEAT_CON_DIALOG)

src/po/vim.pot

@@ -74,7 +74,7 @@ void give_warning(char_u *message, int hl);
 void give_warning_with_source(char_u *message, int hl, int with_source);
 void give_warning2(char_u *message, char_u *a1, int hl);
 void msg_advance(int col);
-void msg_warn_missing_clipboard(bool plus, bool star);
+void msg_warn_missing_clipboard(void);
 int do_dialog(int type, char_u *title, char_u *message, char_u *buttons, int dfltbutton, char_u *textfield, int ex_cmd);
 int vim_dialog_yesno(int type, char_u *title, char_u *message, int dflt);
 int vim_dialog_yesnocancel(int type, char_u *title, char_u *message, int dflt);

src/proto/message.pro

@@ -204,7 +204,7 @@ valid_yank_reg(
     else if (regname == '*' || regname == '+')
     {
 	// Warn about missing clipboard support once
-	msg_warn_missing_clipboard(true, true);
+	msg_warn_missing_clipboard();
 	return FALSE;
     }
 #endif
@@ -1189,7 +1189,7 @@ op_yank(oparg_T *oap, int deleting, int mess)
 	(!clip_plus.available && oap->regname == '+'))
     {
 	oap->regname = 0;
-	msg_warn_missing_clipboard(!clip_plus.available, !clip_star.available);
+	msg_warn_missing_clipboard();
     }
 #endif
 

src/register.c

@@ -729,6 +729,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1927,
 /**/
     1926,
 /**/

src/version.c

@@ -2364,8 +2364,6 @@ typedef struct
 # ifdef FEAT_GUI_HAIKU
     // No clipboard at the moment. TODO?
 # endif
-    // If we've already warned about missing/unavailable clipboard
-    bool did_warn;
 } Clipboard_T;
 #else
 typedef int Clipboard_T;	// This is required for the prototypes.