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

Function arguments tip covers autocompletion if tip doesn't fit in the upper screen space #3821

Closed
DoctorRyner opened this issue Sep 12, 2022 · 4 comments · Fixed by #6488
Closed
Assignees
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@DoctorRyner
Copy link

Summary

Here it's covered and I can't see the autocompletion (but it's there)
Bildschirmfoto 2022-09-12 um 19 39 21

But if I make enough space for it to fit then it works as expected
Bildschirmfoto 2022-09-12 um 19 40 05

Reproduction Steps

No response

Helix log

No response

Platform

macOS

Terminal Emulator

The latest iTerm 2

Helix Version

22.08.1

@DoctorRyner DoctorRyner added the C-bug Category: This is a bug label Sep 12, 2022
@sudormrfbin
Copy link
Member

You can dismiss the signature help popup with Ctrl-c, but showing the completion popup in the upper side if the signature help popup renders below would be a better solution.

@DoctorRyner
Copy link
Author

Oh, thank you for the Ctrl-c, I'll use it as a workaround for the time being

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Sep 14, 2022
@VincentWo
Copy link

This is something that I would like to see fixed and would be willing to contribute, could someone give me pointers where the relevant code lies?

@Pascal-So
Copy link
Contributor

Pascal-So commented Jan 1, 2023

I encountered the same problem today, and for me it only was a problem when the cursor is near the top of the screen. Usually the signature help renders above the cursor and completions below the cursor, but if there is not enough space on top then the signature help moves below the cursor, covering the completions (relevant code here).

I'm not familiar with the codebase but I spent some time browsing today. It looks like the completions are rendered as part of the EditorView component, whereas the signature help is pushed directly to the Compositor as Popup<SignatureHelp>. This means that the signature help will always render on top of the completions, but in my opinion the completions are almost always more important so ideally this would be reversed.

My suggestion would be to separate the completions from EditorView and let them be their independent layer in the compositor. Adding a new layer to the compositor will always put it on top of everything else, which seems like a good fit because the completions popup will usually appear after the signature help popup when writing out function parameters and then disappear again when you're done with one of the function parameters. The signature help can thus remain active in the background and it will be revealed again when it becomes relevant once more as the user wants to see the information for the next parameter. Note that all this is only relevant for the collision case, usually when there's enough space above and below the cursor then this change should have no visible effect.

Another option would be to dynamically close the signature help popup when a collision occurs. This would be beneficial for users with themes where the border between overlapping popups is barely or not at all visible and overlaps could thus be confusing. The problem however is that, as mentioned previously, the user might still want to see the the signature help either when the completions popup has closed or when the view has been scrolled so that a collision-free rendering would again be possible (always speaking from the perspective of a user that relies on the automatic signature help and doesn't invoke it via a command).

Thus I think the ideal solution in my view would be as follows: split the completions into a separate layer as mentioned before, but additionally (and here comes the annoying/tricky bit), rewrite the compositor renderer to render from top to bottom, while keeping track of some kind of boolean alpha value in Buffer. If I understand correctly, we currently render the bottom layer first, and everything after that is drawn on top of the existing buffer, overwriting the now hidden information. Reversing this would have the advantage of letting components further down know if they'd be hidden, and then decide whether they want to adjust their position because of that or maybe skip their rendering stage completely. Then we could rewrite Popup::render() to first check if there is some space available where it could be rendered to still be visible, and if no such space is available, then it simply doesn't get rendered. Because let's face it: when would you ever want to render a partially hidden popup?

I know this sound like an overcomplicated solution to a seemingly simple issue, but I believe that splitting out the copletions into a separate layer makes sense anyway, and then we can still decide later on whether we want to follow through with the rendering reversal or not. In general, I suspect that such collision problems might come up more often in the future, especially once an extension api is added and extensions add their own popups. I therefore believe that it makes sense to invest into a general solution to the problem.

Sorry for the wall of text, and as mentioned before I'm not yet too familiar with the codebase, so please do point out if I misunderstood something. Also, @VincentWo I don't want to steal your show here, so if any of the information I wrote down here seems useful to you then feel free to take it and go ahead on your implementation :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
5 participants