clipboard: disallow recursion; show hint only once (#7203)

- Show hint only once per session.
- provider#clipboard#Call(): prevent recursion
- provider#clear_stderr(): use has_key(), because :silent! is still
  captured by :redir.

closes #7184

4 files changed, 59 insertions, 17 deletions

diff --git a/runtime/autoload/provider.vim b/runtime/autoload/provider.vim
index b46ae12b3c..7439b638c2 100644
--- a/runtime/autoload/provider.vim
+++ b/runtime/autoload/provider.vim
@@ -10,7 +10,9 @@ function! provider#stderr_collector(chan_id, data, event) dict
 endfunction
 
 function! provider#clear_stderr(chan_id)
-   silent! call remove(s:stderr, a:chan_id)
+  if has_key(s:stderr, a:chan_id)
+    call remove(s:stderr, a:chan_id)
+  endif
 endfunction
 
 function! provider#get_stderr(chan_id)
diff --git a/runtime/autoload/provider/clipboard.vim b/runtime/autoload/provider/clipboard.vim
index 8fe53c495a..6454a01c2a 100644
--- a/runtime/autoload/provider/clipboard.vim
+++ b/runtime/autoload/provider/clipboard.vim
@@ -32,7 +32,7 @@ function! s:try_cmd(cmd, ...) abort
   if v:shell_error
     if !exists('s:did_error_try_cmd')
       echohl WarningMsg
-      echomsg "clipboard: error: ".(len(out) ? out[0] : '')
+      echomsg "clipboard: error: ".(len(out) ? out[0] : v:shell_error)
       echohl None
       let s:did_error_try_cmd = 1
     endif
@@ -168,5 +168,13 @@ function! s:clipboard.set(lines, regtype, reg) abort
 endfunction
 
 function! provider#clipboard#Call(method, args) abort
-  return call(s:clipboard[a:method],a:args,s:clipboard)
+  if get(s:, 'here', v:false)  " Clipboard provider must not recurse. #7184
+    return 0
+  endif
+  let s:here = v:true
+  try
+    return call(s:clipboard[a:method],a:args,s:clipboard)
+  finally
+    let s:here = v:false
+  endtry
 endfunction
diff --git a/src/nvim/ops.c b/src/nvim/ops.c
index 6c873a96c0..c01840cfd0 100644
--- a/src/nvim/ops.c
+++ b/src/nvim/ops.c
@@ -55,12 +55,11 @@ static yankreg_T y_regs[NUM_REGISTERS];
 
 static yankreg_T *y_previous = NULL; /* ptr to last written yankreg */
 
-static bool clipboard_didwarn_unnamed = false;
-
 // for behavior between start_batch_changes() and end_batch_changes())
 static int batch_change_count = 0;           // inside a script
 static bool clipboard_delay_update = false;  // delay clipboard update
 static bool clipboard_needs_update = false;  // clipboard was updated
+static bool clipboard_didwarn = false;
 
 /*
  * structure used by block_prep, op_delete and op_yank for blockwise operators
@@ -5541,22 +5540,17 @@ static yankreg_T *adjust_clipboard_name(int *name, bool quiet, bool writing)
   yankreg_T *target = NULL;
   bool explicit_cb_reg = (*name == '*' || *name == '+');
   bool implicit_cb_reg = (*name == NUL) && (cb_flags & CB_UNNAMEDMASK);
-  int save_redir_off = redir_off;
   if (!explicit_cb_reg && !implicit_cb_reg) {
     goto end;
   }
 
   if (!eval_has_provider("clipboard")) {
-    if (batch_change_count == 1 && explicit_cb_reg && !quiet) {
-      redir_off = true;   // Avoid recursion from :redir + emsg().
+    if (batch_change_count == 1 && !quiet
+        && (!clipboard_didwarn || (explicit_cb_reg && !redirecting()))) {
+      clipboard_didwarn = true;
       // Do NOT error (emsg()) here--if it interrupts :redir we get into
       // a weird state, stuck in "redirect mode".
       msg((char_u *)MSG_NO_CLIP);
-    } else if (batch_change_count == 1 && implicit_cb_reg
-               && !quiet && !clipboard_didwarn_unnamed) {
-      redir_off = true;  // Avoid recursion from :redir + emsg().
-      msg((char_u *)MSG_NO_CLIP);
-      clipboard_didwarn_unnamed = true;
     }
     // ... else, be silent (don't flood during :while, :redir, etc.).
     goto end;
@@ -5586,7 +5580,6 @@ static yankreg_T *adjust_clipboard_name(int *name, bool quiet, bool writing)
   }
 
 end:
-  redir_off = save_redir_off;
   return target;
 }
 
diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua
index e6544a8e2e..f66fbf7c94 100644
--- a/test/functional/clipboard/clipboard_provider_spec.lua
+++ b/test/functional/clipboard/clipboard_provider_spec.lua
@@ -90,8 +90,9 @@ describe('clipboard', function()
     basic_register_test()
   end)
 
-  it('`:redir @+>` with invalid g:clipboard shows error exactly once', function()
-    local screen = Screen.new(72, 5)
+  it('`:redir @+>` with invalid g:clipboard shows exactly one error #7184',
+  function()
+    local screen = Screen.new(72, 4)
     screen:attach()
     command("let g:clipboard = 'bogus'")
     feed_command('redir @+> | :silent echo system("cat CONTRIBUTING.md") | redir END')
@@ -99,15 +100,40 @@ describe('clipboard', function()
       ^                                                                        |
       ~                                                                       |
       ~                                                                       |
+      clipboard: No provider. Try ":CheckHealth" or ":h clipboard".           |
+    ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
+  end)
+
+  it('`:redir @+>|bogus_cmd|redir END` + invalid g:clipboard must not recurse #7184',
+  function()
+    local screen = Screen.new(72, 4)
+    screen:attach()
+    command("let g:clipboard = 'bogus'")
+    feed_command('redir @+> | bogus_cmd | redir END')
+    screen:expect([[
       ~                                                                       |
       clipboard: No provider. Try ":CheckHealth" or ":h clipboard".           |
+      E492: Not an editor command: bogus_cmd | redir END                      |
+      Press ENTER or type command to continue^                                 |
     ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
   end)
 
-  it('invalid g:clipboard', function()
+  it('invalid g:clipboard shows hint if :redir is not active', function()
     command("let g:clipboard = 'bogus'")
     eq('', eval('provider#clipboard#Executable()'))
     eq('clipboard: invalid g:clipboard', eval('provider#clipboard#Error()'))
+
+    local screen = Screen.new(72, 4)
+    screen:attach()
+    command("let g:clipboard = 'bogus'")
+    -- Explicit clipboard attempt, should show a hint message.
+    feed_command('let @+="foo"')
+    screen:expect([[
+      ^                                                                        |
+      ~                                                                       |
+      ~                                                                       |
+      clipboard: No provider. Try ":CheckHealth" or ":h clipboard".           |
+    ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
   end)
 
   it('valid g:clipboard', function()
@@ -148,6 +174,19 @@ describe('clipboard', function()
     eq(0, eval("g:clip_called_set"))
   end)
 
+  it('`:redir @+>|bogus_cmd|redir END` must not recurse #7184',
+  function()
+    local screen = Screen.new(72, 4)
+    screen:attach()
+    feed_command('redir @+> | bogus_cmd | redir END')
+    screen:expect([[
+      ^                                                                        |
+      ~                                                                       |
+      ~                                                                       |
+      E492: Not an editor command: bogus_cmd | redir END                      |
+    ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
+  end)
+
   it('has independent "* and unnamed registers by default', function()
     insert("some words")
     feed('^"*dwdw"*P')