From 9882e25dc44f1165e1edc8b3898356e493b6b3fe Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Sun, 20 Aug 2017 02:13:04 +0200
Subject: [PATCH 1/3] clipboard: avoid error flood during :redir

redir_write():
- This is a "batch" operation which was not yet covered by
  start_batch_changes()

adjust_clipboard_name():
- msg() and friends during :redir will, of course, cause redir_write()
  to try to capture that message, which causes recursion.
- EMSG() here is trouble: if it interrupts :redir it is a mess.
  Rather than deal with the mess, show a non-error message.

closes #7182
closes #7184
closes #7183
ref #6048
ref #7032
---
 runtime/autoload/health/provider.vim          | 12 ++--
 runtime/autoload/provider/clipboard.vim       | 18 +++--
 src/nvim/eval.c                               | 18 ++---
 src/nvim/message.c                            |  2 +
 src/nvim/ops.c                                | 69 +++++++++++--------
 .../clipboard/clipboard_provider_spec.lua     | 32 +++++++--
 6 files changed, 98 insertions(+), 53 deletions(-)

diff --git a/runtime/autoload/health/provider.vim b/runtime/autoload/health/provider.vim
index ec20615f69a86a..26db5b77b7c0f7 100644
--- a/runtime/autoload/health/provider.vim
+++ b/runtime/autoload/health/provider.vim
@@ -121,14 +121,14 @@ function! s:check_clipboard() abort
   call health#report_start('Clipboard (optional)')
 
   let clipboard_tool = provider#clipboard#Executable()
-  if empty(clipboard_tool)
+  if exists('g:clipboard') && empty(clipboard_tool)
+    call health#report_error(
+          \ provider#clipboard#Error(),
+          \ ["Use the example in :help g:clipboard as a template, or don't set g:clipboard at all."])
+  elseif empty(clipboard_tool)
     call health#report_warn(
-          \ 'No clipboard tool found. Clipboard registers will not work.',
+          \ 'No clipboard tool found. Clipboard registers (`"+` and `"*`) will not work.',
           \ [':help clipboard'])
-  elseif exists('g:clipboard') && (type({}) != type(g:clipboard)
-        \ || !has_key(g:clipboard, 'copy') || !has_key(g:clipboard, 'paste'))
-    call health#report_error(
-          \ 'g:clipboard exists but is malformed. It must be a dictionary with the keys documented at :help g:clipboard')
   else
     call health#report_ok('Clipboard tool found: '. clipboard_tool)
   endif
diff --git a/runtime/autoload/provider/clipboard.vim b/runtime/autoload/provider/clipboard.vim
index 8eb694e9fa2323..0d36dfcf78f8d7 100644
--- a/runtime/autoload/provider/clipboard.vim
+++ b/runtime/autoload/provider/clipboard.vim
@@ -3,6 +3,7 @@
 " available.
 let s:copy = {}
 let s:paste = {}
+let s:clipboard = {}
 
 " When caching is enabled, store the jobid of the xclip/xsel process keeping
 " ownership of the selection, so we know how long the cache is valid.
@@ -23,7 +24,7 @@ function! s:selection.on_exit(jobid, data, event) abort
   call provider#clear_stderr(a:jobid)
 endfunction
 
-let s:selections = { '*': s:selection, '+': copy(s:selection)}
+let s:selections = { '*': s:selection, '+': copy(s:selection) }
 
 function! s:try_cmd(cmd, ...) abort
   let argv = split(a:cmd, " ")
@@ -55,6 +56,12 @@ endfunction
 
 function! provider#clipboard#Executable() abort
   if exists('g:clipboard')
+    if type({}) isnot# type(g:clipboard)
+          \ || type({}) isnot# get(g:clipboard, 'copy', v:null)
+          \ || type({}) isnot# get(g:clipboard, 'paste', v:null)
+      let s:err = 'clipboard: invalid g:clipboard'
+      return ''
+    endif
     let s:copy = get(g:clipboard, 'copy', { '+': v:null, '*': v:null })
     let s:paste = get(g:clipboard, 'paste', { '+': v:null, '*': v:null })
     let s:cache_enabled = get(g:clipboard, 'cache_enabled', 1)
@@ -104,16 +111,17 @@ function! provider#clipboard#Executable() abort
     return 'tmux'
   endif
 
-  let s:err = 'clipboard: No clipboard tool available. :help clipboard'
+  let s:err = 'clipboard: No clipboard tool. :help clipboard'
   return ''
 endfunction
 
 if empty(provider#clipboard#Executable())
+  " provider#clipboard#Call() *must not* be defined if the provider is broken.
+  " Otherwise eval_has_provider() thinks the clipboard provider is
+  " functioning, and eval_call_provider() will happily call it.
   finish
 endif
 
-let s:clipboard = {}
-
 function! s:clipboard.get(reg) abort
   if s:selections[a:reg].owner > 0
     return s:selections[a:reg].data
@@ -154,7 +162,9 @@ function! s:clipboard.set(lines, regtype, reg) abort
     echohl WarningMsg
     echomsg 'clipboard: failed to execute: '.(s:copy[a:reg])
     echohl None
+    return 0
   endif
+  return 1
 endfunction
 
 function! provider#clipboard#Call(method, args) abort
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index ac22d75a835dc7..d6ee13857a0ea6 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -22775,7 +22775,7 @@ typval_T eval_call_provider(char *provider, char *method, list_T *arguments)
 
 bool eval_has_provider(const char *name)
 {
-#define check_provider(name) \
+#define CHECK_PROVIDER(name) \
   if (has_##name == -1) { \
     has_##name = !!find_func((char_u *)"provider#" #name "#Call"); \
     if (!has_##name) { \
@@ -22791,17 +22791,17 @@ bool eval_has_provider(const char *name)
   static int has_python3 = -1;
   static int has_ruby = -1;
 
-  if (!strcmp(name, "clipboard")) {
-    check_provider(clipboard);
+  if (strequal(name, "clipboard")) {
+    CHECK_PROVIDER(clipboard);
     return has_clipboard;
-  } else if (!strcmp(name, "python3")) {
-    check_provider(python3);
+  } else if (strequal(name, "python3")) {
+    CHECK_PROVIDER(python3);
     return has_python3;
-  } else if (!strcmp(name, "python")) {
-    check_provider(python);
+  } else if (strequal(name, "python")) {
+    CHECK_PROVIDER(python);
     return has_python;
-  } else if (!strcmp(name, "ruby")) {
-    check_provider(ruby);
+  } else if (strequal(name, "ruby")) {
+    CHECK_PROVIDER(ruby);
     return has_ruby;
   }
 
diff --git a/src/nvim/message.c b/src/nvim/message.c
index b90c475ede0afa..fe4cb65779372e 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -2519,6 +2519,7 @@ static void redir_write(const char *const str, const ptrdiff_t maxlen)
   if (redirecting()) {
     /* If the string doesn't start with CR or NL, go to msg_col */
     if (*s != '\n' && *s != '\r') {
+      start_batch_changes();
       while (cur_col < msg_col) {
         if (capture_ga) {
           ga_concat_len(capture_ga, " ", 1);
@@ -2535,6 +2536,7 @@ static void redir_write(const char *const str, const ptrdiff_t maxlen)
         }
         cur_col++;
       }
+      end_batch_changes();
     }
 
     size_t len = maxlen == -1 ? STRLEN(s) : (size_t)maxlen;
diff --git a/src/nvim/ops.c b/src/nvim/ops.c
index 5c6f4d0d07af97..7bbc72b0918891 100644
--- a/src/nvim/ops.c
+++ b/src/nvim/ops.c
@@ -58,8 +58,8 @@ 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 bool clipboard_delay_update = false;  // delay clipboard update
 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
 
 /*
@@ -5524,7 +5524,7 @@ int get_default_register_name(void)
 }
 
 /// Determine if register `*name` should be used as a clipboard.
-/// In an unnammed operation, `*name` is `NUL` and will be adjusted to `'*'/'+'` if
+/// In an unnamed operation, `*name` is `NUL` and will be adjusted to */+ if
 /// `clipboard=unnamed[plus]` is set.
 ///
 /// @param name The name of register, or `NUL` if unnamed.
@@ -5535,33 +5535,44 @@ int get_default_register_name(void)
 /// if the register isn't a clipboard or provider isn't available.
 static yankreg_T *adjust_clipboard_name(int *name, bool quiet, bool writing)
 {
-  if (*name == '*' || *name == '+') {
-    if(!eval_has_provider("clipboard")) {
-      if (!quiet) {
-        EMSG("clipboard: No provider. Try \":CheckHealth\" or "
-             "\":h clipboard\".");
-      }
-      return NULL;
-    }
-    return &y_regs[*name == '*' ? STAR_REGISTER : PLUS_REGISTER];
-  } else if ((*name == NUL) && (cb_flags & CB_UNNAMEDMASK)) {
-    if(!eval_has_provider("clipboard")) {
-      if (!quiet && !clipboard_didwarn_unnamed) {
-        msg((char_u *)"clipboard: No provider. Try \":CheckHealth\" or "
-            "\":h clipboard\".");
-        clipboard_didwarn_unnamed = true;
-      }
-      return NULL;
+#define MSG_NO_CLIP "clipboard: No provider. " \
+  "Try \":CheckHealth\" or \":h clipboard\"."
+
+  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().
+      // 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 (avoid a flood of messages).
+    goto end;
+  }
+
+  if (explicit_cb_reg) {
+    target = &y_regs[*name == '*' ? STAR_REGISTER : PLUS_REGISTER];
+    goto end;
+  } else {  // unnamed register: "implicit" clipboard
     if (writing && clipboard_delay_update) {
       clipboard_needs_update = true;
-      return NULL;
+      goto end;
     } else if (!writing && clipboard_needs_update) {
-      // use the internal value
-      return NULL;
+      goto end;  // use the internal value
     }
 
-    yankreg_T *target;
     if (cb_flags & CB_UNNAMEDPLUS) {
       *name = (cb_flags & CB_UNNAMED && writing) ? '"': '+';
       target = &y_regs[PLUS_REGISTER];
@@ -5569,10 +5580,12 @@ static yankreg_T *adjust_clipboard_name(int *name, bool quiet, bool writing)
       *name = '*';
       target = &y_regs[STAR_REGISTER];
     }
-    return target; // unnamed register
+    goto end;
   }
-  // don't do anything for other register names
-  return NULL;
+
+end:
+  redir_off = save_redir_off;
+  return target;
 }
 
 static bool get_clipboard(int name, yankreg_T **target, bool quiet)
@@ -5740,7 +5753,7 @@ static void set_clipboard(int name, yankreg_T *reg)
   (void)eval_call_provider("clipboard", "set", args);
 }
 
-/// Avoid clipboard (slow) during batch operations (i.e., a script).
+/// Avoid slow things (clipboard) during batch operations (while/for-loops).
 void start_batch_changes(void)
 {
   if (++batch_change_count > 1) {
@@ -5750,7 +5763,7 @@ void start_batch_changes(void)
   clipboard_needs_update = false;
 }
 
-/// Update the clipboard after batch changes finished.
+/// Counterpart to start_batch_changes().
 void end_batch_changes(void)
 {
   if (--batch_change_count > 0) {
diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua
index eb2eeee0da00e5..941112d9aeb121 100644
--- a/test/functional/clipboard/clipboard_provider_spec.lua
+++ b/test/functional/clipboard/clipboard_provider_spec.lua
@@ -4,6 +4,7 @@ local helpers = require('test.functional.helpers')(after_each)
 local Screen = require('test.functional.ui.screen')
 local clear, feed, insert = helpers.clear, helpers.feed, helpers.insert
 local feed_command, expect, eq, eval = helpers.feed_command, helpers.expect, helpers.eq, helpers.eval
+local command = helpers.command
 
 local function basic_register_test(noblock)
   insert("some words")
@@ -80,15 +81,34 @@ local function basic_register_test(noblock)
   expect("two and three and one")
 end
 
-describe('the unnamed register', function()
+describe('clipboard', function()
   before_each(clear)
-  it('works without provider', function()
+
+  it('unnamed register works without provider', function()
     eq('"', eval('v:register'))
     basic_register_test()
   end)
+
+  it('`:redir @+>` with invalid g:clipboard shows error exactly once', function()
+    local screen = Screen.new(72, 8)
+    screen:attach()
+    command("let g:clipboard = 'bogus'")
+    feed_command('redir @+> | :call system("cat CONTRIBUTING.md") | redir END')
+    -- it is made empty
+    screen:expect([[
+      ~                                                                       |
+      ~                                                                       |
+      ~                                                                       |
+      Error detected while processing function provider#clipboard#Executable: |
+      line    5:                                                              |
+      clipboard: invalid g:clipboard                                          |
+      clipboard: No provider. Try ":CheckHealth" or ":h clipboard".           |
+      Press ENTER or type command to continue^                                 |
+    ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
+  end)
 end)
 
-describe('clipboard usage', function()
+describe('clipboard', function()
   local function reset(...)
     clear('--cmd', 'let &rtp = "test/functional/fixtures,".&rtp', ...)
   end
@@ -139,7 +159,7 @@ describe('clipboard usage', function()
     eq({'some\ntext', '\nvery binary\n'}, eval("getreg('*', 1, 1)"))
   end)
 
-  it('support autodectection of regtype', function()
+  it('autodetects regtype', function()
     feed_command("let g:test_clip['*'] = ['linewise stuff','']")
     feed_command("let g:test_clip['+'] = ['charwise','stuff']")
     eq("V", eval("getregtype('*')"))
@@ -169,7 +189,7 @@ describe('clipboard usage', function()
     eq({{' much', 'ktext', ''}, 'b'}, eval("g:test_clip['+']"))
   end)
 
-  it('supports setreg', function()
+  it('supports setreg()', function()
     feed_command('call setreg("*", "setted\\ntext", "c")')
     feed_command('call setreg("+", "explicitly\\nlines", "l")')
     feed('"+P"*p')
@@ -187,7 +207,7 @@ describe('clipboard usage', function()
         ]])
   end)
 
-  it('supports let @+ (issue #1427)', function()
+  it('supports :let @+ (issue #1427)', function()
     feed_command("let @+ = 'some'")
     feed_command("let @* = ' other stuff'")
     eq({{'some'}, 'v'}, eval("g:test_clip['+']"))

From cc7e344f8357d07b1df17df0b322152d5c50739b Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Sun, 20 Aug 2017 18:53:58 +0200
Subject: [PATCH 2/3] clipboard: remove start_batch_changes() in redir_write()

start_batch_changes() doesn't avoid invoking the clipboard
once-per-line, because the loop is actually in ex_echo(), which calls
redir_write() for each message. But we've already entered
start_batch_changes() by then, so that was never the problem.

    redir_write at /home/vagrant/old.neovim/build/../src/nvim/message.c:2523
    msg_puts_attr_len at /home/vagrant/old.neovim/build/../src/nvim/message.c:1600
    msg_outtrans_len_attr at /home/vagrant/old.neovim/build/../src/nvim/message.c:1221
    ex_echo at /home/vagrant/old.neovim/build/../src/nvim/eval.c:19433
    do_one_cmd at /home/vagrant/old.neovim/build/../src/nvim/ex_docmd.c:2242

Trying to defer _explicit_ clipboard updates is difficult.
    :redir @+ | silent echo system('cat foo') | redir END
is essentially equivalent to:
    for l in readfile('foo')
        let @+ .= l
    endfor
We cannot make judgements about when to ignore a script's bad decisions.
start_batch_changes() only works around the case of clipboard=unnamed,
i.e. _implicit_ clipboard updates (`:g/foo/d`).  Not explicit
assignment.
---
 src/nvim/message.c                            |  2 --
 src/nvim/ops.c                                |  7 ++--
 .../clipboard/clipboard_provider_spec.lua     | 32 ++++++++++++++-----
 .../fixtures/autoload/provider/clipboard.vim  |  8 +++++
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/src/nvim/message.c b/src/nvim/message.c
index fe4cb65779372e..b90c475ede0afa 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -2519,7 +2519,6 @@ static void redir_write(const char *const str, const ptrdiff_t maxlen)
   if (redirecting()) {
     /* If the string doesn't start with CR or NL, go to msg_col */
     if (*s != '\n' && *s != '\r') {
-      start_batch_changes();
       while (cur_col < msg_col) {
         if (capture_ga) {
           ga_concat_len(capture_ga, " ", 1);
@@ -2536,7 +2535,6 @@ static void redir_write(const char *const str, const ptrdiff_t maxlen)
         }
         cur_col++;
       }
-      end_batch_changes();
     }
 
     size_t len = maxlen == -1 ? STRLEN(s) : (size_t)maxlen;
diff --git a/src/nvim/ops.c b/src/nvim/ops.c
index 7bbc72b0918891..6c873a96c08f2d 100644
--- a/src/nvim/ops.c
+++ b/src/nvim/ops.c
@@ -5558,7 +5558,7 @@ static yankreg_T *adjust_clipboard_name(int *name, bool quiet, bool writing)
       msg((char_u *)MSG_NO_CLIP);
       clipboard_didwarn_unnamed = true;
     }
-    // ... else, be silent (avoid a flood of messages).
+    // ... else, be silent (don't flood during :while, :redir, etc.).
     goto end;
   }
 
@@ -5567,10 +5567,12 @@ static yankreg_T *adjust_clipboard_name(int *name, bool quiet, bool writing)
     goto end;
   } else {  // unnamed register: "implicit" clipboard
     if (writing && clipboard_delay_update) {
+      // For "set" (copy), defer the clipboard call.
       clipboard_needs_update = true;
       goto end;
     } else if (!writing && clipboard_needs_update) {
-      goto end;  // use the internal value
+      // For "get" (paste), use the internal value.
+      goto end;
     }
 
     if (cb_flags & CB_UNNAMEDPLUS) {
@@ -5772,6 +5774,7 @@ void end_batch_changes(void)
   }
   clipboard_delay_update = false;
   if (clipboard_needs_update) {
+    // unnamed ("implicit" clipboard)
     set_clipboard(NUL, y_previous);
     clipboard_needs_update = false;
   }
diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua
index 941112d9aeb121..08055f61d80845 100644
--- a/test/functional/clipboard/clipboard_provider_spec.lua
+++ b/test/functional/clipboard/clipboard_provider_spec.lua
@@ -93,17 +93,16 @@ describe('clipboard', function()
     local screen = Screen.new(72, 8)
     screen:attach()
     command("let g:clipboard = 'bogus'")
-    feed_command('redir @+> | :call system("cat CONTRIBUTING.md") | redir END')
-    -- it is made empty
+    feed_command('redir @+> | :silent echo system("cat CONTRIBUTING.md") | redir END')
     screen:expect([[
+      ^                                                                        |
+      ~                                                                       |
+      ~                                                                       |
+      ~                                                                       |
       ~                                                                       |
       ~                                                                       |
       ~                                                                       |
-      Error detected while processing function provider#clipboard#Executable: |
-      line    5:                                                              |
-      clipboard: invalid g:clipboard                                          |
       clipboard: No provider. Try ":CheckHealth" or ":h clipboard".           |
-      Press ENTER or type command to continue^                                 |
     ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
   end)
 end)
@@ -118,7 +117,23 @@ describe('clipboard', function()
     feed_command('call getreg("*")') -- force load of provider
   end)
 
-   it('has independent "* and unnamed registers per default', function()
+  it('`:redir @+>` invokes clipboard once-per-message', function()
+    eq(0, eval("g:clip_called_set"))
+    feed_command('redir @+> | :silent echo system("cat CONTRIBUTING.md") | redir END')
+    -- Assuming CONTRIBUTING.md has >100 lines.
+    assert(eval("g:clip_called_set") > 100)
+  end)
+
+  it('`:redir @">` does not invoke clipboard', function()
+    -- :redir to a non-clipboard register, with `:set clipboard=unnamed`.
+    -- Does _not_ propagate to the clipboard (complies with Vim behavior).
+    command("set clipboard=unnamedplus")
+    eq(0, eval("g:clip_called_set"))
+    feed_command('redir @"> | :silent echo system("cat CONTRIBUTING.md") | redir END')
+    eq(0, eval("g:clip_called_set"))
+  end)
+
+  it('has independent "* and unnamed registers per default', function()
     insert("some words")
     feed('^"*dwdw"*P')
     expect('some ')
@@ -325,7 +340,7 @@ describe('clipboard', function()
 
   end)
 
-  describe('with clipboard=unnamedplus', function()
+  describe('clipboard=unnamedplus', function()
     before_each(function()
       feed_command('set clipboard=unnamedplus')
     end)
@@ -369,6 +384,7 @@ describe('clipboard', function()
         really unnamed
         the plus]])
     end)
+
     it('is updated on global changes', function()
       insert([[
 	text
diff --git a/test/functional/fixtures/autoload/provider/clipboard.vim b/test/functional/fixtures/autoload/provider/clipboard.vim
index 411e095c718de7..6d777255c818cf 100644
--- a/test/functional/fixtures/autoload/provider/clipboard.vim
+++ b/test/functional/fixtures/autoload/provider/clipboard.vim
@@ -5,7 +5,13 @@ let s:methods = {}
 let g:cliplossy = 0
 let g:cliperror = 0
 
+" Count how many times the clipboard was invoked.
+let g:clip_called_get = 0
+let g:clip_called_set = 0
+
 function! s:methods.get(reg)
+  let g:clip_called_get += 1
+
   if g:cliperror
     return 0
   end
@@ -19,6 +25,8 @@ function! s:methods.get(reg)
 endfunction
 
 function! s:methods.set(lines, regtype, reg)
+  let g:clip_called_set += 1
+
   if a:reg == '"'
     call s:methods.set(a:lines,a:regtype,'+')
     call s:methods.set(a:lines,a:regtype,'*')

From 88165a798e7459fecf815a13c853949923d4b278 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Sun, 20 Aug 2017 22:17:03 +0200
Subject: [PATCH 3/3] clipboard: test g:clipboard validation, fix a bug

Also fix `:help foo` highlighting in health.vim
---
 runtime/autoload/health.vim                   |  2 +-
 runtime/autoload/provider/clipboard.vim       |  6 ++--
 .../clipboard/clipboard_provider_spec.lua     | 31 ++++++++++++++-----
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/runtime/autoload/health.vim b/runtime/autoload/health.vim
index f875c8b7972588..bd99a0e1042c05 100644
--- a/runtime/autoload/health.vim
+++ b/runtime/autoload/health.vim
@@ -90,7 +90,7 @@ endfunction
 
 " Changes ':h clipboard' to ':help |clipboard|'.
 function! s:help_to_link(s) abort
-  return substitute(a:s, '\v:h%[elp] ([^|][^"\r\n]+)', ':help |\1|', 'g')
+  return substitute(a:s, '\v:h%[elp] ([^|][^"\r\n ]+)', ':help |\1|', 'g')
 endfunction
 
 " Format a message for a specific report item
diff --git a/runtime/autoload/provider/clipboard.vim b/runtime/autoload/provider/clipboard.vim
index 0d36dfcf78f8d7..8fe53c495a6b7a 100644
--- a/runtime/autoload/provider/clipboard.vim
+++ b/runtime/autoload/provider/clipboard.vim
@@ -57,14 +57,14 @@ endfunction
 function! provider#clipboard#Executable() abort
   if exists('g:clipboard')
     if type({}) isnot# type(g:clipboard)
-          \ || type({}) isnot# get(g:clipboard, 'copy', v:null)
-          \ || type({}) isnot# get(g:clipboard, 'paste', v:null)
+          \ || type({}) isnot# type(get(g:clipboard, 'copy', v:null))
+          \ || type({}) isnot# type(get(g:clipboard, 'paste', v:null))
       let s:err = 'clipboard: invalid g:clipboard'
       return ''
     endif
     let s:copy = get(g:clipboard, 'copy', { '+': v:null, '*': v:null })
     let s:paste = get(g:clipboard, 'paste', { '+': v:null, '*': v:null })
-    let s:cache_enabled = get(g:clipboard, 'cache_enabled', 1)
+    let s:cache_enabled = get(g:clipboard, 'cache_enabled', 0)
     return get(g:clipboard, 'name', 'g:clipboard')
   elseif has('mac') && executable('pbcopy')
     let s:copy['+'] = 'pbcopy'
diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua
index 08055f61d80845..e6544a8e2e76d8 100644
--- a/test/functional/clipboard/clipboard_provider_spec.lua
+++ b/test/functional/clipboard/clipboard_provider_spec.lua
@@ -5,6 +5,7 @@ local Screen = require('test.functional.ui.screen')
 local clear, feed, insert = helpers.clear, helpers.feed, helpers.insert
 local feed_command, expect, eq, eval = helpers.feed_command, helpers.expect, helpers.eq, helpers.eval
 local command = helpers.command
+local meths = helpers.meths
 
 local function basic_register_test(noblock)
   insert("some words")
@@ -90,7 +91,7 @@ describe('clipboard', function()
   end)
 
   it('`:redir @+>` with invalid g:clipboard shows error exactly once', function()
-    local screen = Screen.new(72, 8)
+    local screen = Screen.new(72, 5)
     screen:attach()
     command("let g:clipboard = 'bogus'")
     feed_command('redir @+> | :silent echo system("cat CONTRIBUTING.md") | redir END')
@@ -99,12 +100,26 @@ describe('clipboard', function()
       ~                                                                       |
       ~                                                                       |
       ~                                                                       |
-      ~                                                                       |
-      ~                                                                       |
-      ~                                                                       |
       clipboard: No provider. Try ":CheckHealth" or ":h clipboard".           |
     ]], nil, {{bold = true, foreground = Screen.colors.Blue}})
   end)
+
+  it('invalid g:clipboard', function()
+    command("let g:clipboard = 'bogus'")
+    eq('', eval('provider#clipboard#Executable()'))
+    eq('clipboard: invalid g:clipboard', eval('provider#clipboard#Error()'))
+  end)
+
+  it('valid g:clipboard', function()
+    -- provider#clipboard#Executable() only checks the structure.
+    meths.set_var('clipboard', {
+      ['name'] = 'clippy!',
+      ['copy'] = { ['+'] = 'any command', ['*'] = 'some other' },
+      ['paste'] = { ['+'] = 'any command', ['*'] = 'some other' },
+    })
+    eq('clippy!', eval('provider#clipboard#Executable()'))
+    eq('', eval('provider#clipboard#Error()'))
+  end)
 end)
 
 describe('clipboard', function()
@@ -124,16 +139,16 @@ describe('clipboard', function()
     assert(eval("g:clip_called_set") > 100)
   end)
 
-  it('`:redir @">` does not invoke clipboard', function()
-    -- :redir to a non-clipboard register, with `:set clipboard=unnamed`.
-    -- Does _not_ propagate to the clipboard (complies with Vim behavior).
+  it('`:redir @">` does NOT invoke clipboard', function()
+    -- :redir to a non-clipboard register, with `:set clipboard=unnamed` does
+    -- NOT propagate to the clipboard. This is consistent with Vim.
     command("set clipboard=unnamedplus")
     eq(0, eval("g:clip_called_set"))
     feed_command('redir @"> | :silent echo system("cat CONTRIBUTING.md") | redir END')
     eq(0, eval("g:clip_called_set"))
   end)
 
-  it('has independent "* and unnamed registers per default', function()
+  it('has independent "* and unnamed registers by default', function()
     insert("some words")
     feed('^"*dwdw"*P')
     expect('some ')