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

execute() should clear cmdline #6035

Closed
wsdjeg opened this issue Jan 31, 2017 · 15 comments · Fixed by #9338
Closed

execute() should clear cmdline #6035

wsdjeg opened this issue Jan 31, 2017 · 15 comments · Fixed by #9338
Labels
bug issues reporting wrong behavior has:plan ui
Milestone

Comments

@wsdjeg
Copy link
Contributor

wsdjeg commented Jan 31, 2017

  • nvim --version: master
  • Vim (version: ) behaves differently? yes, in vim, it works well
  • Operating system/version: arch
  • Terminal name/version: termite
  • $TERM: xterm-termite

Actual behaviour

execute() do not clear cmdline

Expected behaviour

execute() should clear cmdline

Steps to reproduce using nvim -u NORC

test.vim

let g:testvim_test = 'hello neovim'
let g:testoldversion = 0

fu! Test()
    let list = []
    if has('patch-7.4.2010') && !g:testoldversion
        for var in getcompletion('g:testvim_','var')
            call add(list, var . ' = ' . string(get(g:, var[2:] , '')))
        endfor
    else
        redraw
        for var in filter(map(s:execute('let g:'), "matchstr(v:val, '\\S\\+')"), "v:val =~# '^testvim_'")
            call add(list,'g:' . var . ' = ' . string(get(g:, var , '')))
        endfor
    endif
    return list
endf

function! s:execute(cmd) abort
  if exists('*execute')
    return split(execute(a:cmd), "\n")
  endif

  redir => output
  execute a:cmd
  redir END
  return split(output, "\n")
endfunction

reproduce step:

  1. nvim -u test.vim
  2. :echo Test()[0] get g:testvim_test = 'hello neovim'
  3. :let g:testoldversion = 1
  4. :echo Test()[0] get :echo Test()[0] g:testvim_test = 'hello neovim'
@wsdjeg
Copy link
Contributor Author

wsdjeg commented Jan 31, 2017

this issue is found in SpaceVim/SpaceVim#186

@jamessan
Copy link
Member

Can you test #5975?

@wsdjeg
Copy link
Contributor Author

wsdjeg commented Feb 2, 2017

@jamessan I am not sure if it is some issue with my config, but it still does not work well for me.

@justinmk
Copy link
Member

justinmk commented Feb 3, 2017

Why is this a problem? Avoiding redraws is a good thing. Can you call :redraw?

@justinmk justinmk closed this as completed Feb 3, 2017
@wsdjeg
Copy link
Contributor Author

wsdjeg commented Feb 3, 2017

@justinmk This is different in vim, and redraw does not work, the output still has echo Test() at the begin.

@justinmk justinmk reopened this Feb 3, 2017
@justinmk
Copy link
Member

justinmk commented Feb 3, 2017

Sorry, I misunderstood the report.

@justinmk
Copy link
Member

justinmk commented Feb 3, 2017

I see the problem, this only happens if you :echo Test()[0] as a user from the cmdline. If you add this to the end of the script, then source the script:

:echo Test()[0]

There's no problem. Nor if you store the result in a variable and then echo the variable.

let g:a= Test()[0]

@justinmk justinmk added this to the unplanned milestone Feb 3, 2017
@justinmk justinmk added bug issues reporting wrong behavior ui labels Feb 3, 2017
@wsdjeg
Copy link
Contributor Author

wsdjeg commented Feb 3, 2017

yes, I have trid use let g:a= Test()[0] in my plugin, and it works, but I think this maybe a bug.

@justinmk
Copy link
Member

justinmk commented Feb 3, 2017

It is a bug. It can be worked around by adding :redraw to the beginning of Test() (or anywhere before the call to execute()).

@wsdjeg
Copy link
Contributor Author

wsdjeg commented Feb 3, 2017

I have trid to add redraw at the beginning of Test(), but it does not work, BTW, I am using neovim 0.1.7, and has('patch-7.4.2010') return 0。

@justinmk
Copy link
Member

justinmk commented Feb 3, 2017

Hmm, confirmed. I would guess keep_msg is not being updated for some reason. Even with :redraw!, the column still isn't updated so it looks like this:

                        g:testvim_test = 'hello neovim'

@wsdjeg
Copy link
Contributor Author

wsdjeg commented Mar 13, 2017

Thanks, and a easy way to reproduce is :
:echo type(execute('au'))

you will see
:echo type(execute('au')) 1
instead of
1

@justinmk justinmk changed the title execute should clear cmdline. execute() should clear cmdline Mar 13, 2017
@liushapku
Copy link
Contributor

Today I dug into the source code of both vim and neovim and identified the cause of difference: vim calls msg_col = 0 at the end of function f_execute() while neovim does not do that. Adding a msg_col = 0 to the end of f_execute() simply solves the problem.


Using @wsdjeg's test case I cannot see the problem, perhaps because I ran using nvim -u NONE and no autocmd is installed. I created the following test case which should be saved in a file and then in Ex mode, type :so {the_file}.

call execute('echo 12345678')
echo 'see where I am'

If the "see where I am" starts from col 1, then it is correct and compliant to vim's behavior. nvim will display the string from col 9.

@justinmk justinmk modified the milestones: unplanned, todo, 0.3.3 Dec 6, 2018
@liushapku
Copy link
Contributor

liushapku commented Dec 6, 2018

Acutally, simply set it to 0 will make the following echon behave differently. (vim also behaves incorrectly here)

I created a PR to fully address this issue.

A test script is saved at here. I don't know how to add test to the repo. Each call Testxa() should generate exactly the same result as call Testxb() where x=1,2,3,4.

justinmk pushed a commit to liushapku/neovim that referenced this issue Dec 8, 2018
justinmk pushed a commit to liushapku/neovim that referenced this issue Dec 8, 2018
liushapku added a commit to liushapku/neovim that referenced this issue Dec 8, 2018
@janlazo
Copy link
Contributor

janlazo commented Dec 8, 2018

I tried patching vim-patch:8.1.0569 and vim-patch:8.1.0571 for this issue but the tests fail because assertion with value from screenchar. Anyone who wants to port them should consider lua tests instead if the screenchar tests in src/nvim/testdir/ are required for compatibility.

janlazo pushed a commit to janlazo/neovim that referenced this issue Dec 12, 2018
janlazo pushed a commit to janlazo/neovim that referenced this issue Dec 12, 2018
janlazo pushed a commit to janlazo/neovim that referenced this issue Dec 26, 2018
janlazo pushed a commit to janlazo/neovim that referenced this issue Dec 26, 2018
@justinmk justinmk modified the milestones: 0.4.x, todo Jan 2, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Jan 25, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Jan 25, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Feb 4, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Feb 4, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Mar 9, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Mar 9, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Jun 3, 2019
janlazo pushed a commit to janlazo/neovim that referenced this issue Jun 3, 2019
@justinmk justinmk modified the milestones: todo, 0.4 Jun 3, 2019
hlpr98 pushed a commit to hlpr98/neovim that referenced this issue Jun 17, 2019
hlpr98 pushed a commit to hlpr98/neovim that referenced this issue Jun 17, 2019
hlpr98 pushed a commit to hlpr98/neovim that referenced this issue Jun 17, 2019
hlpr98 pushed a commit to hlpr98/neovim that referenced this issue Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior has:plan ui
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants