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

Disclose pager to half of screen height immediately #9105

Merged
merged 2 commits into from Aug 9, 2022

Conversation

faho
Copy link
Member

@faho faho commented Aug 2, 2022

This removes that bit where we only show 4 rows at most at first,
instead we disclose between half and up to the full terminal height.

This results in less pressing of tab to get the other results, and
better visibility of results.

However, it also means that it'll push the shell up to the top of the
terminal a lot.

So this is an experiment to see how that works in practice.

Fixes #2698


The direct impetus for this is #9089, which creates up to 12 rows. In that case it feels supremely weird to have to disclose "8 more rows", when there's clearly more room.

An alternative is to increase the maximum either up to a constant, or up to half the terminal height.


There's some weirdness here where it doesn't count the pager as fully disclosed, and so pressing tab again doesn't immediately move down into it, which doesn't actually save any key presses. I'd look into it, but only after we've decided which way to go.

(also the constant here still exists and such)


Edit: Originally this went fullscreen immediately. That's awkward because it removes all scrollback.

@faho faho added the RFC label Aug 2, 2022
@krobelus
Copy link
Member

krobelus commented Aug 4, 2022

Is it possible to show exactly the number of rows where we don't push the shell up? That would require knowing the current row which I guess we can't.

Anyway, I think the suggested behavior is okay to try and see if anyone complains. I don't think I use the pager much in day-to-day fishing so I can't really tell if it is bothersome.

Substantially improving the pager is hard.
I'm starting to think it should behave more like in an IDE - where the completion pager has a fixed max size, and perhaps show completions automatically, without having to press tab (though that has security implications).

This removes that bit where we only show 4 rows at most at first,
instead we disclose up to the full terminal height.

This results in less pressing of tab to get the other results, and
better visibility of results.

However, it also means that it'll push the shell up to the top of the
terminal a lot.

So this is an experiment to see how that works in practice.

Fixes fish-shell#2698
This often shows more elements without entirely snapping fish to the
top, which is jarring when it just ZOOMS off.

Of course it will do that after the second press, but then you at
least have some indication.
@faho
Copy link
Member Author

faho commented Aug 8, 2022

I've been trying out a different idea: We disclose between 50% and 100%, but at least 4 rows.

In practice that means we disclose half the terminal, which feels okay and leaves a bit of scrollback on-screen and doesn't snap fish off into the stratosphere (i.e. the top of the screen).

I'm tempted to just merge that and see how people get on with it. No shame in reverting it, and I don't really see RFC PRs working too well.

@faho faho changed the title Disclose pager to screen height immediately Disclose pager to half of screen height immediately Aug 9, 2022
@faho faho added this to the fish 3.6.0 milestone Aug 9, 2022
@faho faho added enhancement and removed RFC labels Aug 9, 2022
@faho
Copy link
Member Author

faho commented Aug 9, 2022

Okay, I'm merging this speculatively. If people don't like it it's extremely easy to back out or refine, but without adding it to master we're not gonna get enough testing.

@faho faho merged commit 7d8009e into fish-shell:master Aug 9, 2022
@faho faho deleted the pager-fullscreen branch September 7, 2022 11:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use more than 4 rows for completions
2 participants