gnvim
gnvim copied to clipboard
vhakulinen
Implement ext messages
Throwing this here for now. It works, but this is not ready to be merged yet. Feel free to test, play around, and do further development on this. Feedback is welcome.
@vhakulinen What are the main features/changes that you want to see in this PR in order for it to be ready for merging? (If you don’t mind, I just want to get an idea of what is specifically wanted here so I can help where I can.)
At least support for msg_show and msg_history_show. For msg_show, it would be nice to have some "special" support for the confirm/prompt messages. msg_showmode, msg_showcmd, and msg_ruler would be most useful within a custom statusbar, so those would be a matter of another PR.
@badosu This PR fixes that indentation issue from #38 for me; if you get the chance, maybe try this branch out and see if the same is true for you. (With this branch I don't need to use reset.) Interested to see why that's the case?
I now have a working draft implementation of support for msg_history_show messages. I've added some explanatory comments (unnecessary in the actual commit) of some of the things I've done. I'm sure there is a better way of unwrapping the messages (as I mentioned in the comments) than what I did, but this is just a draft. Also, there is no 'press ENTER' prompt to remove the messages as there is in the TUI (do we want that or similar in the final version?). Let me know what you think (I've put it into a gist since it's fairly large; also, some of the changes are only from cargo fmt):
Draft commit for msg_history_show message support
I wasn't sure if we should to add a struct specifically for supporting msg_history_show (let me know if you think we should), so I've left it without one for now.
@smolck
Also, there is no 'press ENTER' prompt to remove the messages as there is in the TUI
Look at 'msg_clear', neovim sends that event to clear messages.
For interactive messages looks at 'confirm', 'confirm_sub' and 'return_prompt' here
It's not clear how to clear 'msg_history_show' though, I'd assume it's 'msg_clear' but in the docs it says that it clears only messages shown by 'msg_show'.
Also, there is no 'press ENTER' prompt to remove the messages as there is in the TUI
Also see https://github.com/neovim/neovim/pull/10065.
It's not clear how to clear 'msg_history_show' though, I'd assume it's 'msg_clear' but in the docs it says that it clears only messages shown by 'msg_show'.
Interesting. That's right; I did a few tests, and msg_clear isn't sent after msg_history_show. msg_history_show is only cleared after the next message receives the msg_clear command (in gnvim, that is; in the TUI there is a 'press ENTER' prompt to clear it).
@vhakulinen Will you be able to submit a PR over at neovim-lib for supporting multi grid, that way there will be no need to use a personal fork? It already has ext_message support from badosu’s PR, so multi grid support is all that’s needed. (I don’t want to submit a PR for it if you already will when you get the chance, but if you don’t mind me submitting a PR let me know.)
EDIT: As @badosu pointed out, there is no need for this currently as multigrid is not used here, and the upstream master branch will work; my bad, nevermind this.
@smolck As far as I can see that should not be necessary at the moment, just using the upstream master branch should do it (this PR does not make use of multigrid).
It's not clear how to clear 'msg_history_show' though, I'd assume it's 'msg_clear' but in the docs it says that it clears only messages shown by 'msg_show'.
Currently nvim delegates all choices how to navigate the message history (if it doesn't fit into one screen), how to dismiss it, etc to the client. Somehow running the builtin pager over ext_messages seems hard, nvim doesn't know how much space messages take on the client's screen.
Also see neovim/neovim#10065.
This is a merged PR. Is there any remaining issue with it?
Currently nvim delegates all choices how to navigate the message history (if it doesn't fit into one screen), how to dismiss it, etc to the client.
So we'll need to decide how we want to handle those things? If so, what about having the message history be a scrolled window? When it doesn't fit into one screen, the user could scroll through it, and if he doesn't interact with it, we could make it fade away after a few seconds. I think we'd need to create a new struct for the purpose though (it seems that might be the best option for showing the messages from msg_history_show anyway; it gives us more options). Correct me if I’m wrong on any of this.
This is a merged PR. Is there any remaining issue with it?
I was trying to point out that you need to have recent master of nvim with this. Tho' you need quite recent version of it either way to have ext_messages support.
What about having the message history be a scrolled window?
Thats probably the way to go. I'm not sure about the UX and here I would try to see what other GUIs do.
Opening a split to the bottom might be a decent solution too since many editors (afaik) does so, like so:
That might be a bit confusing regarding vim's other features, such as the quickfix window.
Just a remark: I think the way messages are displayed in neovim are more 'log-like' than 'notification-like'.
I think a pretty straightforward implementation would be a console-like component below the statusbar that displays messages as lines, with some keybindings/UI/behavior (closing, hiding, confirming, etc). This also helps with less distraction potential (which I think is one of the best features of gnvim).
I am not saying the current implementation should adhere to it, just my two cents on this topic.
PS: @vhakulinen beat me up to it, excited to follow progress on this! :+1:
Just a remark: I think the way messages are displayed in neovim are more 'log-like' than 'notification-like'.
Agreed
Following what vhakulinen showed, we would probably want a keybinding to toggle between showing the message history window at the bottom and hiding it. We might also want to make it configurable in where it is positioned (i.e. have a setting in init.vim or as args to the gnvim command that allows for this configuration). Of course, if the user didn't want any of this he could always disable ext_messages with command args to gnvim (which I figure we'll probably want to add as well, just in case the user wants a more minimal GUI experience).
Of course, if the user didn't want any of this he could always disable ext_messages with command args to gnvim
This is somewhat awkward to do at the moment, requires fiddling with .desktop file or running as a command, in the future we might want to improve this.
I created https://github.com/vhakulinen/gnvim/issues/59 to follow up on it.
Experience report: especially long messages gets easily on way of the cursor, which is really annoying. The current solution needs to move them messages up/down according to the cursor position or make the messages narrower (I get full screen wide messages from lsp). Other solution would be to not display the messages on top of the grid. Or some middle ground would work too: some messages are displayed as they are currently in this branch and some on the bottom in its own panel which pushes the grid upwards. Dunno.
Experiences with ext_messages will vary very much based on what plugins one has enabled.
@vhakulinen We might want to check how ext_messages are supported in this branch of gonvim (ext_messages), and/or this one (0.4.0). They might not have support like we’re trying to implement (I haven’t looked into them), but maybe still worth a look to get some ideas.
The current solution needs to move them messages up/down according to the cursor position or make the messages narrower (I get full screen wide messages from lsp).
Moving the messages up/down might be a bit distracting, and making the messages narrower lengthwise would make them less readable. Making them narrower vertically-speaking would only work up until a certain font size, right?
Other solution would be to not display the messages on top of the grid. Or some middle ground would work too: some messages are displayed as they are currently in this branch and some on the bottom in its own panel which pushes the grid upwards.
It seems keeping the messages as they currently are implemented may require unnecessary boilerplate. How about making all (or most) messages from msg_show be at the bottom (or top, side, etc., make it user-configurable) in their own panel that moves the grid accordingly, and that is shown/hidden with a (user-configurable) keybinding? If the panel was hidden by the user through said keybinding, we could show the messages like the TUI does at the bottom of the screen. What would be nice about this is that it would be consistent with how we decided we want to do the msg_history_show support (using a panel at the bottom like other text editors).
With the above in place, msg_history_show might not even be necessary, as we could keep all of the msg_show messages listed in the panel, until they are cleared by the user or too many are shown ('too many' would be a user-configurable number of messages). We may still want a dialog for confirm/prompt messages though. (Let me know if I wasn't clear on any of this.)
@vhakulinen @badosu So, are we decided on how we want to go about implementing this? I’d like to help out where I can, so when you get the chance let me know what we want to do, if anything (assuming we’re decided on this). Also, please see my above comment if you haven’t; I’d like to know you guys’ opinions on the ideas I presented (and re-iterated in the case of the second part) there if you don't mind.
I've not done much with this in a while, and am now focusing my efforts to #95. Own panel/container for the messages on the bottom is probably the best solution. If you want to work on this, you could start by doing that.
Own panel/container for the messages on the bottom is probably the best solution. If you want to work on this, you could start by doing that.
@vhakulinen Okay, thanks! Will hopefully start working on that soon (once I find time for it). Just to confirm, would you prefer that I branch my changes off master and then PR them into this branch, or something else?
Which ever you prefer is fine. There is some base work in this branch which is likely to benefit you, so this branch might be better option.
Which ever you prefer is fine. There is some base work in this branch which is likely to benefit you, so this branch might be better option.
Thanks! I will definitely use this branch as a reference.
@vhakulinen One last thing, would you mind rebasing this branch once you get the chance? After that I’ll branch off it and get started soon (hopefully).
Rebased (I actually had done this a while back on my local already). Made the neovim-lib dependency to point to the upstream one.