Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report an error when using / writing to system clipboard without +clipboard compile flag #14768

Open
delvh opened this issue May 14, 2024 · 5 comments

Comments

@delvh
Copy link

delvh commented May 14, 2024

At the moment, I've often enough had the problem that Vim seems to "magically" not be working initially regarding system clipboard copy-pasting:
For example, when pressing "*y5y, my expectation is that 5 lines are copied to the system clipboard.
However, many distros ship with a minimal vim by default that does not have the +clipboard build flag, most notably Ubuntu.
As such, to an end user, it simply looks like Vim is broken.

I often enough used Vim on a new Ubuntu system, and wondered why the command above simply does nothing.
As such, I would recommend adding an error when attempting to access the system registers "* and "+ when that compile flag is missing.
Preferably, this would also include read-only access to the clipboard, but even write-access only would already be a huge improvement.

@chrisbra
Copy link
Member

Preferably, this would also include read-only access to the clipboard, but even write-access only would already be a huge improvement.

I am not sure I understand this comment and I am not sure this is actually possible. clipboard handling is a feature provided by the X Server, so you need a connection to X11 Server in order to be able to make use of the clipboard features here.

@delvh
Copy link
Author

delvh commented May 14, 2024

Yes, but I mean displaying an error when Vim was not built with +clipboard but trying to access the system clipboard (mostly the commands yank and paste, but also whenever you try to write to a custom register, i.e. "*daw).
That is something we can statically determine as we simply need to listen for if the "* and "+ registers should be used.
As per my understanding, when the compile flag is missing, these registers simply turn into another Vim-internal register.
However, that's not what users expect when using these registers.

I am not talking about checking whether the X Server exists (although it would of course also be nice to display an error if the text could not be copied for any other reason such as the X Server not being available).

@chrisbra
Copy link
Member

As per my understanding, when the compile flag is missing, these registers simply turn into another Vim-internal register.

No, Vim falls back to the 0 register in that case.

@delvh
Copy link
Author

delvh commented May 15, 2024

Yes, that's what I mean.
I have nothing against keeping this behavior.
Nevertheless, I still recommend displaying an error in that case instead:
That seems to be much more "expected behavior" to me than silently falling back to something else than the user requested.
As I said, as a user, my intuition in this case is that I'm requesting the system clipboard, not anything else.

In the worst case, there's the possibility of adding a config option that is enabled by default whether to show an error (enabled) or use the old behavior (disabled).
However, I doubt there will be many users for it as the purpose of these registers should be known by users already.

@chrisbra
Copy link
Member

How about the following patch:

diff --git a/src/clipboard.c b/src/clipboard.c
index 8b9850e44..6c766047e 100644
--- a/src/clipboard.c
+++ b/src/clipboard.c
@@ -2220,10 +2220,12 @@ adjust_clip_reg(int *rp)
            *rp = ((clip_unnamed_saved & CLIP_UNNAMED_PLUS)
                                           && clip_plus.available) ? '+' : '*';
     }
-    if (!clip_star.available && *rp == '*')
-       *rp = 0;
-    if (!clip_plus.available && *rp == '+')
+    if ((!clip_star.available && *rp == '*') ||
+           (!clip_plus.available && *rp == '+'))
+    {
+       msg_warn_missing_clipboard();
        *rp = 0;
+    }
 }

 #endif // FEAT_CLIPBOARD
diff --git a/src/message.c b/src/message.c
index 03c7072a7..216b76192 100644
--- a/src/message.c
+++ b/src/message.c
@@ -55,6 +55,9 @@ static int msg_hist_len = 0;
 static FILE *verbose_fd = NULL;
 static int  verbose_did_open = FALSE;

+static int  did_warn_clipboard = FALSE;
+static char *warn_clipboard = "Clipboard register not available, using register 0";
+
 /*
  * When writing messages to the screen, there are many different situations.
  * A number of variables is used to remember the current state:
@@ -4060,6 +4063,20 @@ msg_advance(int col)
            msg_putchar(' ');
 }

+
+/*
+ * Warn about missing Clipboard Support
+ */
+    void
+msg_warn_missing_clipboard()
+{
+    if (!did_warn_clipboard)
+    {
+       msg(_(warn_clipboard));
+       did_warn_clipboard = TRUE;
+    }
+}
+
 #if defined(FEAT_CON_DIALOG) || defined(PROTO)
 /*
  * Used for "confirm()" function, and the :confirm command prefix.
diff --git a/src/proto/message.pro b/src/proto/message.pro
index 6657a08ec..54a0a7765 100644
--- a/src/proto/message.pro
+++ b/src/proto/message.pro
@@ -73,6 +73,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(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);
diff --git a/src/register.c b/src/register.c
index 47ed21846..5b61e1da0 100644
--- a/src/register.c
+++ b/src/register.c
@@ -199,6 +199,12 @@ valid_yank_reg(
 #endif
                                                        )
        return TRUE;
+    // Warn about missing clipboard support once
+    else if (regname == '*' || regname == '+')
+    {
+       msg_warn_missing_clipboard();
+       return FALSE;
+    }
     return FALSE;
 }

@@ -1164,10 +1170,12 @@ op_yank(oparg_T *oap, int deleting, int mess)
        return OK;

 #ifdef FEAT_CLIPBOARD
-    if (!clip_star.available && oap->regname == '*')
-       oap->regname = 0;
-    else if (!clip_plus.available && oap->regname == '+')
+    if ((!clip_star.available && oap->regname == '*') || (!clip_plus.available
+               && oap->regname == '+'))
+    {
        oap->regname = 0;
+       msg_warn_missing_clipboard();
+    }
 #endif

     if (!deleting)                 // op_delete() already set y_current

chrisbra added a commit to chrisbra/vim that referenced this issue May 17, 2024
Problem:  Missing Clipboard support may get un-noticed
Solution: Warn when trying to access register +/* and clipboard
          support has not been compiled in or is not working

TODO: needs test

related: vim#14768

Signed-off-by: Christian Brabandt <cb@256bit.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants