-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
[RFC] ui: implement ext_messages to externalize all messages in msg area #7466
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
Conversation
This would be great for Oni! I'd like to explore some new ways of rendering messages in the UI. |
5f2ca51
to
c8ef189
Compare
So, this should basically work for early testing. Multiple Currently these is no concept of "multi-line messages", the API should probably try to recover it where it is possible, for instance single multiline I have only tested |
8cd6b25
to
a4e882c
Compare
Rebased. Also changed:
|
Some more progress of this (partly because I think I understand a bit more how message.c works, and partly because this will be useful for #8455 and #8707).
which is probably closer to what UI:s want (rather than to how the msg_ semi-abstraction is constructed internally). The intention behind the "keep_batch" parameter is to signal if the message should continue to build up a batch of messages,( It is not clear however how it should work when you type Without this feature,
The latter is somewhat of a break of the current external UI design, but there a more situations where synchronous UI calls would be useful, such as client interpreted movement commands in #8707 and potentially first class support of dialogs. |
How about calling this
I'm hoping for a third approach: remove the blocking nature of "press RETURN" entirely. I've been experimenting with this in a local branch, and it works nicely.
Messages should be passive and opt-in, instead of constant in-your-face panic. |
Maybe. This is still at the design phase, rather than the name bikeshedding phase :P Also the semantics are currently the reverse of buf_lines.
It would be nice to see this branch.
I think my post was pretty clear in giving clients control, which implies giving users control (by configuring their UI, or using the UI that fits their use case). Clients being able to reimplement |
Yes but the "somewhat of a break of the current external UI design" would be good to avoid, I guess. And working well by default is preferable. |
I don't think I have or would argue against this :) My point is that The purpose of
As I already mentioned this will be very useful for other stuff. It would be the most straightforward way to implement |
Hmm. If UI defines the layout, this is circular. What happens for |
How is it circular? #8707 prototypes this today: nvim asks the UI for what window is below a specific handle, and it replies with the relevant window handle. Then nvim enters that window, exactly as if it has queried an internal datastructure as it does without ext_windows. |
I see nothing in |
5be4d98
to
431fcde
Compare
Thanks for the latest commits and the helpful docs. Sorry, I didn't realize that I can also verify that |
ad0618d
to
0c53ae5
Compare
Multiline messages can now be concretely tested with lua:
|
1960318
to
15f515e
Compare
I think we should merge this, so it doesn't sit around forever. This PR now implements the MVP of letting the UI control the display of the message instead of grid. More fine grained control of the flow will be a follow-up (possibley based on feedback from UI implementers using the MVP). |
5202c45
to
77eb680
Compare
src/nvim/message.c
Outdated
@@ -1124,6 +1163,24 @@ void set_keep_msg(char_u *s, int attr) | |||
keep_msg_attr = attr; | |||
} | |||
|
|||
void msg_set_ext_kind(const char *msg_kind) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
msg_ext_set_kind
More generally, some of these flags seem potentially useful for the legacy message system. It's not clear to me if they should be considered "external only".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, their names are not public or anything, they could be changed later. (Also it is an useful ambiguity that ext
also can mean "extended". If lua will render internal popupmenu from "ext" events, ext_popupmenu
then means "extended popupmenu")
Co-Author: Dongdong Zhou <dzhou121@gmail.com>
@@ -392,7 +398,7 @@ function Screen:expect(expected, attr_ids, attr_ignore) | |||
.. ') differs from configured height(' .. #actual_rows .. ') of Screen.' | |||
end | |||
for i = 1, #actual_rows do | |||
if expected_rows[i] ~= actual_rows[i] then | |||
if expected_rows[i] ~= actual_rows[i] and expected_rows[i] ~= "{IGNORE}|" then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bfredl should this IGNORE feature be mentioned in the Screen:expect docstring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seconded, I wasn't aware of it, and it will be pretty useful to me 👍 Although, I have to admit, I'd probably not have seen the docstring as well..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feel free to add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also some ext keys are missing, but that is not the same kind of deal: print_snapshot()
will add them, so one doesn't need to know about them to use them.
extracted from #5686. Mostly just a heads-up I'm looking into this, might need some redesign before it is ready to be tested/used.