Add command to open a popup as a new buffer

[jerabaul29]

closes #8748

The idea is to make it possible to press ctrl-o to open the popup content in a new buffer. Once this work, I can easily add ctrl-h and ctrl-v to open in horizontal and vertical splits :) .

This is still an early WIP; what remains to be done:

• create the new buffer as read only to avoid the need to use ! to exit etc
• find how to grab the content of the popup to put it into the new buffer
• find how to grab a meaningful title for the popup to put it as the new buffer title
• find how to enable the correct coloring of the new buffer

I will look into this more in the days to come :) .

[jerabaul29]

For now, starting from a popup, this opens a new buffer with a preset title and content. This compiles fine.

[jerabaul29]

I am trying to understand the structure of the classes that would be involved in getting this to work, and if changes should be made, what is the best way. Looking in particular into https://github.com/helix-editor/helix/blob/master/helix-term/src/ui/lsp.rs#L50 and related.

So it looks like the popup content: T where T: Component has no way to get the actual string-like content of the popup from the content as it is now; all it can do is ask the content to render itself.

I see 2 ways further then:

• ask the content to render itself in my new buffer, rather than putting a string in the new buffer; but then, I guess moving around, copy pasting, etc, will not work? Then this would not be so useful. actually I do not think I could / should ask for a render at this location in the code: it can be done only from higher up in the app.

• add some Options<ContentAsStrings> or similar to the Popup struct, to contain, if relevant (not all popups are of a nature that makes them relevant to know their content to make it possible to open in a new buffer; i.e., a long documentation is relevant to open in a new buffer; a list of selections or other hoovers, likely not), the title and main body content and language information of a popup content. I.e., this would mean, in order to be able to open for example the documentation of a function in a new popup, to change the popup to being:

pub struct ContentAsString {
    title: String,
    body: String,
    language: String,
}

pub struct Popup<T: Component> {
    contents: T,
    position: Option<Position>,
    margin: Margin,
    size: (u16, u16),
    child_size: (u16, u16),
    position_bias: Open,
    scroll: usize,
    auto_close: bool,
    ignore_escape_key: bool,
    id: &'static str,
    has_scrollbar: bool,
    // fields used in case the Popup can be opened in a new buffer; e.g., lsp documentation
    content_as_strings: Option<ContentAsStrings>,
}

and then this content_as_string could be set to Some only if relevant in the Popup::new, or by default set to None.

With this information, it would be trivial to open for example doc popups in a new buffer.

Would this make sense from an architecture point of view? Is it worth to give it a try?

[jerabaul29]

This is now able to open doc into a new markdown buffer :) .

[jerabaul29]

This now has the following functionality:

• starting from e.g. this:

• I can now press ctrl-o to get to:

The main things I do not manage to get at the current time:

• making the file really read-only from helix viewpoint; I tried setting readonly to true, but helix still allows to edit the content of the buffer
• getting a more meaningful title for the new buffer; for example doc-println! in this case; I can easily replace "hover" with any string, but I have no idea how I could get that I am getting information about the println! element from the context I have.

Any advice on these 2 points (and the general draft of course :) ) would be welcome; @the-mikedavis any advice? :)

[jerabaul29]

(of course refactoring, supporting splits etc, will come soon, just trying to find how to do the last "blocker actions" before refactoring and extending :) ).

[the-mikedavis]

I don't intend on properly looking at or merging this feature until #5505 is possible. With #5505 we could make this possible only on popups where this makes sense, like hover, and we could cleanly downcast the type to a concrete Popup<Markdown>, where we would be able to read the contents. Without it we need to duplicate the contents and store it on the Popup as you have here with StringsContent which IMO is too hacky.

[the-mikedavis]

The path of a buffer isn't meant to be a name - this should be left as a scratch buffer instead

[jerabaul29]

Yes, I agree / understand this is hacky... But at the same time, I would like to have a more illustrative buffer name than "scratch", so that if I have several docs open, I can navigate easily between them. I.e. instead of having "scratch", "scratch", "scratch", I would find it much easier to navigate between several docs named "doc_NAME1", "doc_NAME2", "doc_NAME3". Any idea / recommendation / "non hacky way" you would do this?

[the-mikedavis]

No, these docs shouldn't be opened for very long if they're just being used for reference anyways so I think scratch is fine

[the-mikedavis]

The new document should be created with Document::from and then added to the editor with Editor::new_file_from_document (which is currently private but could be pub). That will remove the need to modify the selection, jumplist and last-saved-revision.

[the-mikedavis]

This is set for files that have readonly permissions. A readonly "mode" for buffers that prevents editing isn't implemented yet

[jerabaul29]

Ok, so I guess we could consider implementing some form of "make readonly" function? Should I open a new issue / PR to discuss? Any recommended direction to implement something like this? :)

[the-mikedavis]

See #7128. I suspect that it would be a large change and it needs some design before anyone goes about making a PR that attempts it

[the-mikedavis]

This passes the C-o event to the contained component (a Markdown in hover's case) which isn't necessary. The popup is handling the event so it shouldn't pass it down. Closing the popup is done below by returning Some(close_fun)

[jerabaul29]

and we could cleanly downcast the type to a concrete Popup, where we would be able to read the contents

Interesting; is there already some "machinery" / tooling in place for this downcast + getting the contents from it (could you point me to it if so), or is it something that would be implemented following #5505 , or something to be implemented after it? :)

[the-mikedavis]

Here's a block that finds and downcasts a component in the compositor into a Overlay<Picker<T>> or Overlay<DynamicPicker<T>>:

helix/helix-term/src/ui/picker.rs

Lines 477 to 482 in 2661e05

	let picker = match compositor.find::<Overlay<Self>>() {
	Some(Overlay { content, .. }) => Some(content),
	None => compositor
	.find::<Overlay<DynamicPicker<T>>>()
	.map(|overlay| &mut overlay.content.file_picker),
	};

The compositor uses std::any::Any for dynamically typing the components.