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

Switch statusline & prompt #5882

Closed
wants to merge 1 commit into from
Closed

Conversation

Fl1tzi
Copy link

@Fl1tzi Fl1tzi commented Feb 8, 2023

#5758

This switches the position of the statusline with the position of the prompt to improve readability of the statusline while the prompt is opened.

TODO:

  • split not rendering correctly

@CptPotato
Copy link
Contributor

I think this should have a config option. I personally prefer the current layout.

@Fl1tzi Fl1tzi changed the title Switch statusline & prompt [NOT READY] Switch statusline & prompt Feb 8, 2023
@goyalyashpal
Copy link
Contributor

goyalyashpal commented Feb 8, 2023

will self hide this as resolved


[NOT READY]

u can mark it as draft

image

@Fl1tzi also, add "Close " before the issue number in the description.

@Fl1tzi Fl1tzi marked this pull request as draft February 8, 2023 14:57
@Fl1tzi Fl1tzi changed the title [NOT READY] Switch statusline & prompt Switch statusline & prompt Feb 8, 2023
@goyalyashpal
Copy link
Contributor

goyalyashpal commented Feb 9, 2023

having this as a config option will make themes inconsistent.

  • do u prefer current just due to habit or is there an actual functionality that it has (which proposed one doesn't have)?
  • have u tried the proposed layout?
  • the current layout comes with so much habit being used in neo/vi/m lineage. so, it's natural that a change will feel odd...

but for the proposed layout, there's actually a functionality concern - to avoid the blockage of status bar from the popups of command line


i don't think helix should adhere to borrowed and backward habits - the biggest habit it challenged is by picking its own keybind in a totally different way from n/vi/m and users who gave up on that n/vi/m are happily embracing it...

so, having habit as a point of discussion against functionality - especially for such a meager thing is not....

@howard36
Copy link
Contributor

howard36 commented Feb 9, 2023

Alternatively, we could just move the popup one line up, so that it appears above both the status line and command line.

Pros:

  • Readability: Popup does not cover the status and command line
  • Habit: Status line above command line is consistent with vim/neovim layout, so users switching to Helix will be familiar with it

Cons:

  • Command popup isn't as close to the command line - related components in a UI should generally be near each other. Personally I think having an extra line in between won't change much, but it'd be best to actually test this layout first.

@goyalyashpal
Copy link
Contributor

Just stumbled upon one more pro for having the command line on top:

  • if the errors in executing a command are long, then expansion of command line can be a viable option for future.
  • currently, if command line were to span multiple lines - it will again - be very unpleasant with status bar.

Example:

  • try typing :cd C:\Prog <tab> <ret>
  • i.e. execute command :cd C:\"Program Files"

It will show this 128 character long error with helix 22.12 (96ff64a8)

Couldn't change the current working directory: The filename, directory name, or volume label syntax is incorrect. (os error 123)

@quomat
Copy link

quomat commented Feb 13, 2023

This is a breaking UI change. This should be escalated to a comnunity discussion.
While I see this as 100% benefitial, I understand it breaks the layout loved by many users and will probably be rejected, to my regret :(
Having config option just for this is also confusing, it would be better to have a robust layout configuration, when you could e.g. hide the status bar completely (think zen mode) just as with the upper bufferline.

@@ -447,7 +447,7 @@ impl Prompt {
text.render(inner, surface, cx);
}

let line = area.height - 1;
let line = area.height - 2;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the following change on line 642 break the file picker (and probably other prompts as well), since they can underflow.
If you substitute this and the change on line 642 with saturating_sub(2) this seems to fix it.

.area
.clip_top(view.area.height.saturating_sub(1))
.clip_bottom(1); // -1 from bottom to remove commandline
let statusline_area = view.area.clip_top(view.area.height);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces the issue with split views. The easiest solution I could come up with was something like this:

// if we're not at the bottom of the screen, draw the statusline higher, otherwise
// it'll overlap with the next view
let statusline_area = if (viewport.bottom() - 1) == view.area.bottom() {
    view.area.clip_top(view.area.height)
} else {
    view.area
        .clip_top(view.area.height.saturating_sub(1))
        .clip_bottom(1)
};

@debajyoti1990
Copy link

Convenient as it might be for some users, in my humble opinion this should not be default, rather a config option.

@ghc0
Copy link

ghc0 commented Apr 21, 2023

Also it should be customizable to show the status line on the top line of screen or other widgets attached on screen.

Also its show or not could be switched by hot key.

Reasons are as follows:

  1. when use a laptop, the surface of bottom line on screen has a big angle to eye, it is not on surface of keyboard, it is another surface.
  2. Move it to top line is more convenient since its info frequently checked. Or some other place like right position of screen, and use left shift+ right shift to display or not display them as a switch.

@jhugs
Copy link

jhugs commented Jun 21, 2023

What's the status on this? New to helix, and I'd definitely like to be able to see the statusline while typing in the prompt. How / where / or whether it's a config option I don't feel strongly about, I just want to be able to see it

@pascalkuthe
Copy link
Member

I don't think we are going forward with this, see discussion in #7635

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants