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

fix(util): suppress 'press ENTER' in echo_messages() #1668

Merged
merged 1 commit into from
Mar 19, 2020

Conversation

sam-mccall
Copy link
Contributor

This is used for workspace/showMessage, when it contains multiple lines.
Using echomsg in a loop means that each line after the second prompts
before displaying.

The downside of this change is if there are multiple consecutive calls
to echo_messages(), only the last one is shown. (All will appear in :messages)

Motivating use: clangd has "dump AST" code action behind the
-hidden-features flag. This action calls showMessage with a payload like:

FunctionDecl 0x14387ee101e8 </home/sammccall/test.cc:1:1, line:3:1> line:1:6 foo 'void ()'
`-CompoundStmt 0x14387ee10380 <col:11, line:3:1>
  `-DeclStmt 0x14387ee10368 <line:2:3, col:10>
      `-VarDecl 0x14387ee10300 <col:3, col:7> col:7 bar 'int'

This is used for workspace/showMessage, when it contains multiple lines.
Using `echomsg` in a loop means that each line after the second prompts
before displaying.

The downside of this change is if there are multiple consecutive calls
to echo_messages(), only the last one is shown. (All will appear in :messages)

Motivating use case: clangd has "dump AST" code action behind the
-hidden-features flag. This action calls showMessage with a payload like:
```
FunctionDecl 0x14387ee101e8 </home/sammccall/test.cc:1:1, line:3:1> line:1:6 foo 'void ()'
`-CompoundStmt 0x14387ee10380 <col:11, line:3:1>
  `-DeclStmt 0x14387ee10368 <line:2:3, col:10>
      `-VarDecl 0x14387ee10300 <col:3, col:7> col:7 bar 'int'
```
@chemzqm
Copy link
Member

chemzqm commented Mar 19, 2020

I can't reproduce 'press ENTER' behavior, we use :echom to make sure user can get it from :messages.

@chemzqm chemzqm closed this Mar 19, 2020
@sam-mccall
Copy link
Contributor Author

sam-mccall commented Mar 19, 2020

I can't reproduce 'press ENTER' behavior

@chemzqm What did you try? Running :call coc#util#echo_messages(...) directly won't reproduce it, as vim defers "press ENTER" if the command-line is open.

This should do it though:
:call timer_start(1000, { tid -> coc#util#echo_messages('', ['foo','bar','baz'])})

we use :echom to make sure user can get it from :messages.

This patch still does that, it uses echom to send the message to :messages (with no prompt), and additionally echo to print all the lines at once (with a single prompt)

@fannheyward
Copy link
Member

:call timer_start(1000, { tid -> coc#util#echo_messages('', ['foo','bar','baz'])})

doesn't reproduce press ENTER.

@sam-mccall
Copy link
Contributor Author

You're right, sorry was AFK. let me get a proper reproducer.

@sam-mccall
Copy link
Contributor Author

sam-mccall commented Mar 19, 2020

OK, so far the only way I've been able to reproduce this is with an actual language server sending a multiline workspace/showMessage.

e.g. with coc-clangd:

  • add "clangd.arguments": ["-hidden-features"] to coc-settiongs.json
  • create a file with void foo() { int bar; }
  • at start-of-file, :call CocActionAsync('codeAction','')
  • select "Dump FunctionDecl AST"

The patch definitely fixes this for vim 8.1.2269.

It seems I should be able to create the same effect by calling echo_messages from either an autocmd or a timer, but I can't and I'm not sure why.

@fannheyward
Copy link
Member

@sam-mccall no press ENTER in nvim with your test case.

@sam-mccall
Copy link
Contributor Author

sam-mccall commented Mar 19, 2020

@fannheyward Indeed, I got nvim set up and verified it's broken in a different way than vim :-)

master with this patch
vim 8.1.2269 'press ENTER' after each line shows all lines at once
nvim v0.5.0-418-g87d892afa displays only the last line shows all lines at once

(Both with an empty config other than coc.nvim, coc-clangd, coc-settings.json as described above, and set nocompatible | syntax on for vim)

@fannheyward
Copy link
Member

Reproduced, I prefer to add this patch.

@chemzqm in this case, workspace.showMessage('aa\nbb\ncc\ndd') will only show dd on master, with this patch, will show the full message.

@fannheyward fannheyward reopened this Mar 19, 2020
@fannheyward fannheyward merged commit d9ff0e5 into neoclide:master Mar 19, 2020
chemzqm added a commit that referenced this pull request Mar 19, 2020
@sam-mccall sam-mccall deleted the multiline branch March 19, 2020 11:00
@sam-mccall
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants