[RFC] hlsearch/incsearch screen tests + convert test 63

[bfredl]

work in progress. Should also test matchadd/pos/ as its implementation is quite entangled with search_hl (ref #1841 )

[bfredl]

also converting test 63 as it is a bit related, but there is some issues. Apparently a vimscript function can both throw an error and at the same time (as if nothing happened) return an ordinary value, and I don't know how to check that using vim_eval (which must either return an error or a value, but not both) or similar.

[justinmk]

Apparently a vimscript function can both throw an error and at the same time (as if nothing happened) return an ordinary value

Yes, I believe that's true, @ZyX-I mentioned it before.

I think the lua vim_eval should multi-return a (result, error) pair. In most tests the error value will just be ignored. Any tests that were looking for the error message will need to be updated.

[elmart]

Apparently a vimscript function can both throw an error and at the same time (as if nothing happened) return an ordinary value

Didn't know that. But... then, what would the control flow be? I mean, how are you supposed to manage such a thing?

[bfredl]

@elmart That, exactly, is the question. apparently :echo matchdelete(51) + 10 evals to 9 (and also prints the error), so an error doesn't interrupt expression evaluation. Now what happens if you assign this to a let (or sideeffecty function) inside a try? I don't see any "intuitive" answer.

[bfredl]

@justinmk I think vim_eval still does the "sensible" default (if there is an error, the result is typically -1 or something else boring), but there should be a vim_try_eval or something which doesn't turn eval errors into msgpack errors and returns a tuple as you suggested (but perhaps, still turns syntax errors into msgpack errors)

[justinmk]

Not sure if I'm missing something but if vim_eval returns a (result, error) pair, the error part can be ignored on assignment:

result = helpers.vim_eval(...)

Why would we need a separate vim_try_eval method?

[bfredl]

No i meant vim_eval should still throw an error (i e signal an msgpack
error) while vim_try_eval would return the error (which can be igored as
you say) I dont think just igoring errors (and manually have to check for
errors) would be a sensible default in most situations
Den 12 feb 2015 23:06 skrev "Justin M. Keyes" notifications@github.com:

Not sure if I'm missing something but if vim_eval returns a (result,
error) pair, the error part can be ignored on assignment:

result = helpers.vim_eval(...)

Why would we need a separate vim_try_eval method?

—
Reply to this email directly or view it on GitHub
#1930 (comment).

[justinmk]

Oh, good point.

[bfredl]

More trouble on the same theme: the following causes the test to just freeze

local helpers = require('test.functional.helpers')
local eval, clear, execute, expect = helpers.eval, helpers.clear, helpers.execute
local expect, eq, neq = helpers.expect, helpers.eq, helpers.neq

describe('test the tests', function()
  setup(clear)

  it('is working', function()
    execute("let r1 = setmatches([{'group': 'MyGroup1', 'pattern': 'TODO', 'priority': 10, 'id': 1}, {'group': 'MyGroup2', 'pattern': 'FIXME', 'priority': 10, 'id': 1}])")
    eq(0, eval("r1")) -- freezes

    -- more tests follow
  end)
end)

The freeze is actually on the eq line, as if r1 somehow got "poisoned"; remove it and it will just continue with the rest of the test (in the same nvim instance)

[bfredl]

Problem was a "Press ENTER to continue" that blocked the event loop. (weird, as the same problem ought to have been at execute("let r2 = matchdelete(42)"); eq(-1, eval('r2')) above)
I suppose one could just use nvim_command for commands that expect to fail, but it would have been nice if execute was able to detect this condition somehow (instead of just waiting indefinitely)

[bfredl]

Now it it working, but those feed('<cr>') look like an indeterminacy waiting to happen. Is there some reliable way to just turn off Press Enter or press key to continue ? (or should it considered a bug that this condition freezes event processing ? ) I can't use pcall(function() command(...)) as it interrupts evaluation. Or perhaps it is ok to leave it a TODO until vim_try_eval or similar is implemented?

[justinmk]

Is there some reliable way to just turn off Press Enter or press key to continue ?

No. But if there were, the tests would not simulate the user experience...

[bfredl]

Good point. But do you know if execute('...'); feed('<cr>') where the <cr> dissmisses a message caused by the execute (a single call) is potentially indeterministic or not? (i e one could rely on the <cr> not being consumed before the prompt is shown)

[bfredl]

When I think of it more, nothing should potentially consume input between the final <cr> of the execute and the error prompt, but it would be good if someone could confirm it...

[justinmk]

I haven't seen any flakiness related to that in the 30 tests I have been working on. But I do not know for sure.

[bfredl]

Ok, removed the TODOs as the current implementation is closest to the legacy test (and probably to realistic user experience). Marked RFC, but marvim didn't seem to notice...

[bfredl]

I added a small cleanup to screen_basic_test I forgot to do when working on #1883

[bfredl]

Rebased and fixed some whitespace.

[bfredl]

Rebased after #2101 . ping @tarruda

[tarruda]

👍 LGTM

I'm not sure if you are aware, but test hangs can be debugged more easily if you attach the pygtk UI by adding a Screen.debug() statement at the beginning of the test(or anywhere before the hang happens)

[bfredl]

@tarruda Thanks, I think I saw it in the code but I have not tested it yet.

It would be good if events are processed in the "press ENTER" state later on, but until then, I think feed('<cr>') is the simplest solution.

[bfredl]

And given constant screen size it should be deterministic which error messages require this prompt, so this should be safe.

[tarruda]

It would be good if events are processed in the "press ENTER" state later on, but until then, I think feed('') is the simplest solution.

That probably is the main reason I want to eventually refactor the user dialog/message interface to use the terminal abstraction being implemented in #2076. If we remove most "block the world' prompt code, then nvim will always be in a state where it can process vimscript and consequently there will be no need for this distinction between deferred and non deferred events.

[bfredl]

@tarruda Bump. Something missing here, or could this be merged?

[justinmk]

looking now