From 9882e25dc44f1165e1edc8b3898356e493b6b3fe Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sun, 20 Aug 2017 02:13:04 +0200 Subject: 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 --- .../clipboard/clipboard_provider_spec.lua | 32 ++++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'test/functional') diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua index eb2eeee0da..941112d9ae 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['+']")) -- cgit From cc7e344f8357d07b1df17df0b322152d5c50739b Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sun, 20 Aug 2017 18:53:58 +0200 Subject: 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. --- .../clipboard/clipboard_provider_spec.lua | 32 ++++++++++++++++------ .../fixtures/autoload/provider/clipboard.vim | 8 ++++++ 2 files changed, 32 insertions(+), 8 deletions(-) (limited to 'test/functional') diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua index 941112d9ae..08055f61d8 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 411e095c71..6d777255c8 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,'*') -- cgit From 88165a798e7459fecf815a13c853949923d4b278 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sun, 20 Aug 2017 22:17:03 +0200 Subject: clipboard: test g:clipboard validation, fix a bug Also fix `:help foo` highlighting in health.vim --- .../clipboard/clipboard_provider_spec.lua | 31 ++++++++++++++++------ 1 file changed, 23 insertions(+), 8 deletions(-) (limited to 'test/functional') diff --git a/test/functional/clipboard/clipboard_provider_spec.lua b/test/functional/clipboard/clipboard_provider_spec.lua index 08055f61d8..e6544a8e2e 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 ') -- cgit