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

Customizable/configurable status line #2434

Merged
merged 24 commits into from
Jul 18, 2022

Conversation

usagi-flow
Copy link
Contributor

Summary

Building upon the feedback of #2420 and the discussion in #708, I refactored the status line implementation and made it configurable.

The configuration happens in the user's configuration file and distinguishes between three areas of the status line: left, center and right.

[ ... ... LEFT ... ... | ... ... ... ... CENTER ... ... ... ... | ... ... RIGHT ... ... ]

Within each area, status line elements can be defined. There are no particular restrictions regarding the order, and elements can occur multiple times or be removed completely.

The following elements can currently be configured (see helix_view::editor::StatusLineElement):

mode
spinner
file-name
file-encoding
file-type
diagnostics
selections
position

Example

This would move the file name to the center, and add a file type indicated to the far right):

[editor.status-line]
left = ["mode", "spinner"]
center = ["file-name"]
right = ["diagnostics", "selections", "position", "file-encoding", "file-type"]

image

Additional info

The default configuration keeps the same status line layout with the same elements as before. While I do see room for improvement here, I assume it'd better be done in another PR.

On top of the elements which were already present, I added the implementation for one additional element: the file type (disabled by default).

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

This is great! I apologize it took me a while to review.

helix-term/src/ui/statusline.rs Outdated Show resolved Hide resolved
@mtoohey31
Copy link
Contributor

Thanks for working on this, I just started using these changes and I'm loving the customizability already!

Not sure whether I should bring this up now, or wait until this is merged (or if this is even the right place for this discussion...), but I've written a "scope" element here that uses the language server's textDocument/documentSymbol method to display the struct/function/etc. that the cursor is inside, inspired by SmiteshP/nvim-navic. It's still a little rough; right now the icons are hard-coded to nerd-fonts icons, and I'm just triggering the request along with completion on idle, which might not be the right way to go about it, but cleaning things up shouldn't be too much work.

Is that something we'd be interested in supporting? And if so, should I hold on until after this is merged then create a separate PR, or is there another way I should go about things?

@sudormrfbin
Copy link
Member

@mtoohey31 I would recommend a separate PR that adds it as a new component; also we would want to avoid using nerd fonts by default (see #2211).

@mtoohey31
Copy link
Contributor

@mtoohey31 I would recommend a separate PR that adds it as a new component; also we would want to avoid using nerd fonts by default (see #2211).

Sounds good, I'll wait until this is merged then create one. And yep, I'll make sure to default to symbols that standard fonts will support. Thanks!

@sbromberger
Copy link
Contributor

sbromberger commented Jun 21, 2022

For the file type, could you make it configurable? I like seeing file type info but I don't want to take up multiple characters to show it. Having a config option that could override this default would allow me to use a glyph/icon without making assumptions about anyone else's font.

@usagi-flow
Copy link
Contributor Author

usagi-flow commented Jun 22, 2022

For the file type, could you make it configurable? I like seeing file type info but I don't want to take up multiple characters to show it. Having a config option that could override this default would allow me to use a glyph/icon without making assumptions about anyone else's font.

Hmm, without introducing a dependency to devicons (or similar) in Helix, you would basically have to map filetypes to alternative labels (these can be single devicons) in your config; something like this:

[editor.status-line.file-type]
rust = "<devicon-here>"
javascript = "js"

It'd definitely be easy, but I think I'd first gather some extra input.
E.g. maybe we'd be open to an opt-in (i.e. disabled by default) configuration option devicons = "true". This would then allow us to map filetypes for you, and it would have the advantage of enabling devicons at many other places as well; not just in the statusline.

Either way, I believe it's worth opening a separate issue/discussion for this.

@sbromberger
Copy link
Contributor

I'm currently blocked on #2862 (and, by extension, #2831). It'd be great to get this merged.

helix-view/src/editor.rs Outdated Show resolved Hide resolved
helix-term/src/ui/statusline.rs Outdated Show resolved Hide resolved
helix-term/src/ui/statusline.rs Outdated Show resolved Hide resolved
helix-term/src/ui/statusline.rs Outdated Show resolved Hide resolved
helix-view/src/editor.rs Outdated Show resolved Hide resolved
@sudormrfbin
Copy link
Member

sudormrfbin commented Jun 26, 2022

Could you add a section to the docs at https://github.com/helix-editor/helix/blob/master/book/src/configuration.md#editor ?

@usagi-flow
Copy link
Contributor Author

usagi-flow commented Jul 1, 2022

Thank you for the review, @sudormrfbin ! I extended the book a bit to include some configuration documentation (000a742 & 6699c05).
I corrected/changed the code according to the review as well; I do have one open remark regarding trait aliases, see above.

I'll soon rebase my commits because while this PR was open, #2676 was opened and merged, and conflicts with this PR's changes.

@usagi-flow
Copy link
Contributor Author

Alright, I rebased the branch and reintegrated the last few bits of #2676 in 8d28f95. All looks good and clippy is happy! :)

Because none of the functions need `Self` and all of them are in an own
file, there is no explicit need for the struct.
@usagi-flow
Copy link
Contributor Author

As a friendly side note: I don't mind refactoring more of this after the PR was merged, even if it's just cosmetic code changes! :)

Not that I mind doing so now, but we're starting to block some other features which build upon this, and there's always the risk that we have to rebase onto more changes like #2676.

The configuration was ignored after reintegrating the changes of #2676
in 8d28f95.
Comment on lines +69 to +82
// Left side of the status line.

let element_ids = &context.editor.config().statusline.left;
element_ids
.iter()
.map(|element_id| get_render_function(*element_id))
.for_each(|render| render(context, write_left));

surface.set_spans(
viewport.x,
viewport.y,
&context.parts.left,
context.parts.left.width() as u16,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

These parts seemed to be duplicated a bit. Is it better to refactor it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, do you mean something like introducing a function like the following and calling it in render()?

fn render_area(element_ids: &Vec<StatusLineElement>, write: F,  x: u16, y: y16)
    where F: Fn(&mut RenderContext, String, Option<Style>) + Copy

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Tested and looks fine, I just have one nit about the styling not matching current master

helix-term/src/ui/statusline.rs Outdated Show resolved Hide resolved
helix-term/src/ui/statusline.rs Show resolved Hide resolved
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Tested, works great! Thanks for working on this :)

@archseer archseer merged commit dbf68e0 into helix-editor:master Jul 18, 2022
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
* feat(statusline): add the file type (language id) to the status line

* refactor(statusline): move the statusline implementation into an own struct

* refactor(statusline): split the statusline implementation into different functions

* refactor(statusline): Append elements using a consistent API

This is a preparation for the configurability which is about to be
implemented.

* refactor(statusline): implement render_diagnostics()

This avoid cluttering the render() function and will simplify
configurability.

* feat(statusline): make the status line configurable

* refactor(statusline): make clippy happy

* refactor(statusline): avoid intermediate StatusLineObject

Use a more functional approach to obtain render functions and write to
the buffers, and avoid an intermediate StatusLineElement object.

* fix(statusline): avoid rendering the left elements twice

* refactor(statusline): make clippy happy again

* refactor(statusline): rename `buffer` into `parts`

* refactor(statusline): ensure the match is exhaustive

* fix(statusline): avoid an overflow when calculating the maximal center width

* chore(statusline): Describe the statusline configurability in the book

* chore(statusline): Correct and add documentation

* refactor(statusline): refactor some code following the code review

Avoid very small helper functions for the diagnositcs and inline them
instead.
Rename the config field `status_line` to `statusline` to remain
consistent with `bufferline`.

* chore(statusline): adjust documentation following the config field refactoring

* revert(statusline): revert regression introduced by c0a1870

* chore(statusline): slight adjustment in the configuration documentation

* feat(statusline): integrate changes from helix-editor#2676 after rebasing

* refactor(statusline): remove the StatusLine struct

Because none of the functions need `Self` and all of them are in an own
file, there is no explicit need for the struct.

* fix(statusline): restore the configurability of color modes

The configuration was ignored after reintegrating the changes of helix-editor#2676
in 8d28f95.

* fix(statusline): remove the spinner padding

* refactor(statusline): remove unnecessary format!()
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
* feat(statusline): add the file type (language id) to the status line

* refactor(statusline): move the statusline implementation into an own struct

* refactor(statusline): split the statusline implementation into different functions

* refactor(statusline): Append elements using a consistent API

This is a preparation for the configurability which is about to be
implemented.

* refactor(statusline): implement render_diagnostics()

This avoid cluttering the render() function and will simplify
configurability.

* feat(statusline): make the status line configurable

* refactor(statusline): make clippy happy

* refactor(statusline): avoid intermediate StatusLineObject

Use a more functional approach to obtain render functions and write to
the buffers, and avoid an intermediate StatusLineElement object.

* fix(statusline): avoid rendering the left elements twice

* refactor(statusline): make clippy happy again

* refactor(statusline): rename `buffer` into `parts`

* refactor(statusline): ensure the match is exhaustive

* fix(statusline): avoid an overflow when calculating the maximal center width

* chore(statusline): Describe the statusline configurability in the book

* chore(statusline): Correct and add documentation

* refactor(statusline): refactor some code following the code review

Avoid very small helper functions for the diagnositcs and inline them
instead.
Rename the config field `status_line` to `statusline` to remain
consistent with `bufferline`.

* chore(statusline): adjust documentation following the config field refactoring

* revert(statusline): revert regression introduced by c0a1870

* chore(statusline): slight adjustment in the configuration documentation

* feat(statusline): integrate changes from helix-editor#2676 after rebasing

* refactor(statusline): remove the StatusLine struct

Because none of the functions need `Self` and all of them are in an own
file, there is no explicit need for the struct.

* fix(statusline): restore the configurability of color modes

The configuration was ignored after reintegrating the changes of helix-editor#2676
in 8d28f95.

* fix(statusline): remove the spinner padding

* refactor(statusline): remove unnecessary format!()
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
* feat(statusline): add the file type (language id) to the status line

* refactor(statusline): move the statusline implementation into an own struct

* refactor(statusline): split the statusline implementation into different functions

* refactor(statusline): Append elements using a consistent API

This is a preparation for the configurability which is about to be
implemented.

* refactor(statusline): implement render_diagnostics()

This avoid cluttering the render() function and will simplify
configurability.

* feat(statusline): make the status line configurable

* refactor(statusline): make clippy happy

* refactor(statusline): avoid intermediate StatusLineObject

Use a more functional approach to obtain render functions and write to
the buffers, and avoid an intermediate StatusLineElement object.

* fix(statusline): avoid rendering the left elements twice

* refactor(statusline): make clippy happy again

* refactor(statusline): rename `buffer` into `parts`

* refactor(statusline): ensure the match is exhaustive

* fix(statusline): avoid an overflow when calculating the maximal center width

* chore(statusline): Describe the statusline configurability in the book

* chore(statusline): Correct and add documentation

* refactor(statusline): refactor some code following the code review

Avoid very small helper functions for the diagnositcs and inline them
instead.
Rename the config field `status_line` to `statusline` to remain
consistent with `bufferline`.

* chore(statusline): adjust documentation following the config field refactoring

* revert(statusline): revert regression introduced by c0a1870

* chore(statusline): slight adjustment in the configuration documentation

* feat(statusline): integrate changes from helix-editor#2676 after rebasing

* refactor(statusline): remove the StatusLine struct

Because none of the functions need `Self` and all of them are in an own
file, there is no explicit need for the struct.

* fix(statusline): restore the configurability of color modes

The configuration was ignored after reintegrating the changes of helix-editor#2676
in 8d28f95.

* fix(statusline): remove the spinner padding

* refactor(statusline): remove unnecessary format!()
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.

None yet

9 participants