-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Make overlays (e.g. pickers) fullscreen #12311
base: master
Are you sure you want to change the base?
Conversation
The status line should not be hidden. It contains important information like which file I am currently in |
Do you need it while a picker is open? What about just closing the picker to look at the status line and opening back the last picker? Picker/preview gets an extra line this way but I can see the value of status line. |
Yes, why would you want to close the picker just to see the file name? I often glance at the statusline to know where I am currently |
Because it allows an extra line for previews and/or pickers. Edit: I admittedly don't look at the status line much so I do have a bias, let's let the maintainers decide. |
It looks like some form of option C seems to be the popular choice at least in this thread: overlay size shouldn't be a nuclear option. Option A and B are always effectively achievable with option C, and we will do it such that option Z people won't see any difference unless they explicitly go ahead and add the new config. The debate hasn't been this simple obviously but we have to conclude this eventually. |
I have been thinking about the tabs / status line visibility issue, maybe individual config for 4-side padding isn't that bad after all? If any side's padding is config'ed between 0.0 to 1.0 as a float, then the number works as the ratio of full terminal height/width. If it is given as a positive integer or 0, then it is taken as the number of rows/columns. If for some or all sides padding is not specified, old defaults are used. Following config will leave 1 and 2 lines for tabs and status line respectively being fullscreen otherwise: [editor.picker.padding]
top = 1
right = 0
bottom = 2
left = 0 Old default will be: (Not exactly, as [editor.picker.padding]
top = 0.05
right = 0.05
bottom = 0.05
left = 0.05 Weird combinations are possible whether it is good or bad: [editor.picker.padding]
top = 3
right = 0.4
bottom = 0
left = 20 Of course, we can do away with the concept of floats vs. integers and have only one or the other. This was just an idea, comments appreciated. Problem to discuss is: tabs / status line visibility. |
Personally, for my workflow only, I am most okay top to bottom with the following:
Did I miss anything? |
Picker/preview ratio will be another PR as @darshanCommits suggested. |
Ratio can be of full-height / full-height minus status line / full-height minus tabs / full-height minus status line minus tabs based on a separate flag. |
@the-mikedavis Replying to relevant part of #12369 (comment) here, this PR definitely provides usability improvements. We can make this a simple toggle between:
I am okay with this^ too if that makes the merge fast. Edit:
|
I'm not interested in adding config for this in any form. Can we instead set small constants that expand the overlay to cover the full width and/or height (minus statusline) when the terminal is small? |
Yes. What terminal width/height values should trigger this behavior? I also want picker/preview ratio to change to 3:5 on this trigger. Will it be okay? |
The picker/preview ratio part I'm unsure of - some pickers have a few columns and the table can take up quite a bit of horizontal space depending on the current results. #9647 left improvement of the way the columns are laid out as a follow-up and maybe we can improve the way we share space with the preview as well. There's also an idea like #7783 to split the results and preview in a vertical layout that might compete with this. For the overlay part - what size are you at that you'd like a full screen? |
|
Very fair points. We can skip changing picker/preview ratio in this PR then.
I suppose we can target anything below ~200-250 columns. I am suggesting a higher value because we should go as high as possible until the point made in #12311 (comment) becomes a problem if there isn't anything else stopping us. This can essentially make fullscreen the default for a majority of the people though. |
`patchy` is a tool which makes it easy to declaratively manage personal forks by automatically merging pull requests. Check it out here: https://github.com/NikitaRevenco/patchy
`patchy` is a tool which makes it easy to declaratively manage personal forks by automatically merging pull requests. Check it out here: https://github.com/NikitaRevenco/patchy
`patchy` is a tool which makes it easy to declaratively manage personal forks by automatically merging pull requests. Check it out here: https://github.com/NikitaRevenco/patchy
`patchy` is a tool which makes it easy to declaratively manage personal forks by automatically merging pull requests. Check it out here: https://github.com/NikitaRevenco/patchy
`patchy` is a tool which makes it easy to declaratively manage personal forks by automatically merging pull requests. Check it out here: https://github.com/NikitaRevenco/patchy
@the-mikedavis Hi, are there any changes required? |
Maybe change it to 200-ish columns instead of 240?
|
The logic behind a larger value is to maximize preview width. |
I agree with maximizing preview width on the small terminal sizes, but 240 columns is hardly a small terminal where preview can't fit much content :) Here's how current picker looks right now on 4K 27" screen, with 150% scaling and a bit bigger font size due to eyestrain, in 220 columns wide terminal: |
240 columns is definitely not a small terminal but the idea is not to switch on small terminals but to switch when the preview width will start being smaller (approx. half i.e. 120 columns in 240 columns case) (80 column days are gone imo as another commenter pointed). In your second screenshot, you can see that one line is right at the edge and a bit longer line will overflow: Plus, I think you don't face the issue of moving the eyes too much across the screen on 240 columns. If it is a problem then we can certainly go lower. What do you say @the-mikedavis ?
PS: This is my setup too recently. |
when I look at the screenshots of where the overlay starts vs. the beginning of the line with code, i think the readability argument doesn't really exist. |
I couldn't open a PR before for some reason so I made #11780 but here it is now. Open to feedback. Here's how it looks:
Thanks!