Powerlevel9k / powerlevel9k

Powerlevel9k was a tool for building a beautiful and highly functional CLI, customized for you. P9k had a substantial impact on CLI UX, and its legacy is now continued by P10k.
https://github.com/romkatv/powerlevel10k
MIT License
13.46k stars 949 forks source link

[Bugfix] Fix completion menu #1335

Closed dritter closed 4 years ago

dritter commented 5 years ago

This PR is a backport from P10K and fixes several problems around multi line prompts.

Fixes #1267 , #1291 and #1306 .

Test are not green right now, because I am waiting for #1329 to be merged..

romkatv commented 4 years ago

Are you sure it fixes #1267, #1291 and #1306? Can you reproduce these bugs without this PR and then verify that bugs no longer trigger with the PR?

Syphdias commented 4 years ago

Are you sure it fixes #1267, #1291 and #1306? Can you reproduce these bugs without this PR and then verify that bugs no longer trigger with the PR?

issue broken on next fixed on fix-completion-menu
#1267 yes yes
#1291 yes yes
#1306 yes kinda

image

romkatv commented 4 years ago

Are you sure it fixes #1267, #1291 and #1306? Can you reproduce these bugs without this PR and then verify that bugs no longer trigger with the PR?

issue broken on next fixed on fix-completion-menu

1267 yes yes

1291 yes yes

1306 yes kinda

image

I may not be reading the code right but doesn't it simply remove support for P9K_RPROMPT_ON_NEWLINE=false? If this is the goal, there is no need to look at transient_rprompt and to hook zle widgets -- zsh will do the right thing automatically.

dritter commented 4 years ago

I may not be reading the code right but doesn't it simply remove support for P9K_RPROMPT_ON_NEWLINE=false?

D0h! Fixed three Issues and brought back another one.. 🙄 Checking for a fix. Thanks for the hint!

romkatv commented 4 years ago

Moving in the right direction :+1:

You might want to link to https://www.reddit.com/r/zsh/comments/cgbm24/multiline_prompt_the_missing_ingredient/.

dritter commented 4 years ago

Btw. This is still not finished, as transient_rprompt with multiline prompt eats up one line.. Still investigating this.

romkatv commented 4 years ago

Btw. This is still not finished, as transient_rprompt with multiline prompt eats up one line.. Still investigating this.

Does the same thing work in p10k? If it doesn't, I'll join in debugging.

dritter commented 4 years ago

Nah. It works in p10k. It's just a bit hard to read the generated code.. ;)

romkatv commented 4 years ago

Nah. It works in p10k. It's just a bit hard to read the generated code.. ;)

Tell me about it :grin:

It's also a good idea to check that reset-prompt doesn't eat (or create) a line. Hit esc-x and type reset-prompt. Many kinds of bugs can do this sort of badness.

Another test is to hit ctrl-r and to verify that your cursor doesn't move. Same thing -- it's easy to break this.

dritter commented 4 years ago

Thanks for the tips. I'll double check that, once I am this far..

dritter commented 4 years ago

I think this is now done. @romkatv The checks you suggested pass on my machine.

Additionally this resolves issues with ZLE_RPROMPT_INDENT, which we handle as 1 when set to zero.

@Syphdias I even reactivated testUsingUnsetVariables. :) It would be super nice if you could retest this.