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

[RFC] feature/status_components #233

Open
wants to merge 37 commits into
base: dev
Choose a base branch
from

Conversation

Consolatis
Copy link
Contributor

@Consolatis Consolatis commented Jan 26, 2018

I implemented a new StatusBar architecture, the end result is you have status components which can be exposed by any module and used by any statusbar.

The system consists of following classes:

  • StatusBar
  • StatusBarManager
  • StatusComponent
  • StatusComponentFill
  • StatusComponentShim
  • StatusComponentGenerator

The StatusBarManager tries to call get_components() for each loaded module which may return a list of (name, StatusComponent class). It then instantiates each component and stores the instance in an internal dict. If a module does not offer get_components() but offers the old interface (options["status"], get_status) it will wrap those functions in a StatusComponentShim.

StatusBar is responsible for rendering a specific statusbar. The constructor gets a reference to a curses window, a reference to app and a key which defines it's section inside the config. Using the key it gets and parses it's component config string and requests the component instances from StatusBarManager.

A derived StatusComponent class provides (among others) following properties

  • unicode .text
  • curses .style (usually set via ColorManager so it doesn't have to be a curses style)
  • cached .cells (wcswidth)

which will be used by StatusBar.render(). It also provides a compute() function which gets called on every render() tick. compute() allows the component to update it's internal state and returns an opaque value (a serial in the default implementation). The caller can then compare this return value to the one it got at the previous tick and if it differs the component has been changed. This allows the caller to not do anything at all if none of the components report a change.

A derived StatusComponentGenerator class provides a compute() function which works the same as in the StatusComponent class but does neither provide a .text nor .style property. Instead, it provides a get_components() function which returns a list of own StatusComponent instances. This is useful for e.g. the filelist where each sub component may have it's own .style property.

StatusComponentFill will not be instantiated but serves as a placeholder for rendering calculations. The renderer will calculate the unused terminal cell width (using component.cells property) and distribute it evenly between the fill components. This allows a component config string like "fill clock fill" to be rendered with a single clock in the middle of the statusbar.

ui.py instantiates StatusBarManager which offers a add() function which creates new StatusBar instances. ui.py is responsible to add statusbars using this method but only interacts with the manager which in turn interacts with the statusbar instances.

On ui.py refresh() StatusBarManager.render() is called which in turn calls .render() for all StatusBar instances it is responsible for.

My personal config contains this definition of status bars:

"display": {
    "status_top": {
        "components": "lintlogo editor_name editor_version fill filelist fill clock"
    },
    "status_bottom": {
        "components": "app_status fill document_position"
    }
}

Lintlogo is actually quite useful :)
It is the usual editor logo but if a file contains lint errors it changes color to red and appends the number of lint errors.

The default config looks like this:

    "display": {
        "show_top_bar": true,
        "status_top": {
            "components": "editor_logo editor_name editor_version fill filelist fill clock",
            "fillchar": " ",
            "spacechar": " ",
            "truncate": "left",
            "default_align": "left"
        },
        "show_bottom_bar": true,
        "status_bottom": {
            "components": "app_status fill document_position Cursors cursor_count lint",
            "fillchar": " ",
            "spacechar": " ",
            "truncate": "right",
            "default_align": "left"
        },
        ..
    }

If a user wants to change it (e.g. remove the cursor count and move the lint error count to the left) all s/he needs to do is something like:

    "display": {
        "status_bottom": {
            "components": "lint app_status fill document_position"
        }
    }

I primarily made this for my own usage but would like to upstream those changes after cleaning up and polishing it a bit if the design sounds reasonable.

As this branch requires PRs #218 and #221 I based it on a local dev-next branch but will rebase it once they are merged into dev.

Things to do:

  • add StatusComponent truncate priorities
  • add config option for filelist to only show up to x files (+ something like [y more files])
  • resolve all FIXMEs and TODOs
  • figure out why statusbars don't invalidate state after config_changed event
  • fix statusbar invalidation after prompt exit
  • restore default of "fancy logo"
  • respect use_unicode config flag
  • remove direct use of curses module outside of ColorManager and StatusBar (e.g. AppIndicatorPosition)
  • define default components for top and bottom bars (help required)
  • rebase on dev
  • squash

@Consolatis
Copy link
Contributor Author

Updated description. See also docstrings inside statusbar.py.

@richrd
Copy link
Owner

richrd commented Feb 2, 2018

If a module does not offer get_components() but offers the old interface (options["status"], get_status) it will wrap those functions in a StatusComponentShim.

Love this compatibility 👍

Seems like a rather thought out and solid implementation. I'll test it tomorrow and check out the other needed PR's first. Thanks :)

@richrd
Copy link
Owner

richrd commented Feb 3, 2018

I found a slight bug: when lots of files are opened and they don't fit in the top bar, the clock is still shown correctly but the left most items are hidden and some files aren't visible.

I tried it with this:

python3 suplemon.py suplemon/*.py

I love the coloring though, makes everything a lot more structured and readable.

EDIT:
Another one I found is that if I edit the config in suplemon and save, the top bar disappears. As soon as a key is pressed it reappears. I'm not sure if you've noticed but when the user config file is saved within suplemon it reloads the config automatically. Maybe that's causing some trouble.

@Consolatis
Copy link
Contributor Author

Consolatis commented Feb 3, 2018

I found a slight bug: when lots of files are opened and they don't fit in the top bar, the clock is still shown correctly but the left most items are hidden and some files aren't visible

This is because of the truncate=left setting for top_bar.
If you set it to truncate=right it will instead cut off the clock and still show the editor logo name and version.

There is another setting which may be more useful for those cases though:

"modules": {
    "filelist": {
        "rotate": true
    }
}

See modules/filelist.py get_default_config()
I wonder if we could switch that setting on automatically if the statusbar detects it needs to truncate but that would leak the state of a component into the generic statusbar renderer..

@richrd
Copy link
Owner

richrd commented Feb 3, 2018

Ah, ok! I'd forgot that the current version of suplemon doesn't handle that. The entire status line is just truncated if it's too long... It would be nice to somehow limit the file list length as it's probably potentially the longest item. Not sure what the best solution could be though.

@Consolatis
Copy link
Contributor Author

Consolatis commented Feb 3, 2018

Maybe it might make sense to default to filelist rotate: true and top_bar truncate: right so at least the current file will always be visible? Or keep the default for those like it is now but mention it in readme / internal help?

Personally I prefer the non rotated filelist but you are right, if the screen is too small (e.g. a small tmux pane) or there are too many files open you won't even see which file is selected.

@Consolatis
Copy link
Contributor Author

Consolatis commented Feb 3, 2018

Or maybe.. StatusComponents could provide another flag which allows them to indicate if they can be hidden in case of truncating?

Then truncating would first go through all the components and check that flag and if set remove the component. And only if there still is not enough room to render the remaining ones start removing the components from the left/right (based on setting) regardless of that flag until the bar is small enough.

Edit:
Or maybe instead of it being a simple boolean flag it could actually be a integer priority. The higher the priority (app_status, current file) the less likely it is they will be truncated.

It would be nice to somehow limit the file list length as it's probably potentially the longest item

This would actually be a nice feature for the filelist module, regardless of the truncating issue.
Not based on the actual length of the entry but the count of files. Some config which says only show the currently active file + the two following files and an additional "file" which just states [$x more files] or something along those lines.

Edit2: implemented

@Consolatis
Copy link
Contributor Author

Consolatis commented Feb 4, 2018

Another one I found is that if I edit the config in suplemon and save, the top bar disappears. As soon as a key is pressed it reappears. I'm not sure if you've noticed but when the user config file is saved within suplemon it reloads the config automatically. Maybe that's causing some trouble.

Yes I noticed that as well, StatusBar binds to the config changed event to reload its settings but for some reason the state is not invalidated (or it is actually invalidated, then rendered and finally erased by some other part). Once you do something to invalidate a bar (like changing or switching to another file (filelist), moving the cursor (document_position) or resizing the window (both bars)) it will get redrawn. This needs investigation and is listed in "Things to do" as

figure out why statusbars don't invalidate state after config_changed event

Edit:
should be fixed, curses screen.erase() required an additional screen.noutrefresh() before writing further content, otherwise the erase seems to be delayed until the next call to noutrefresh() / refresh() and thus erase anything modified up to that point.

@Consolatis
Copy link
Contributor Author

Consolatis commented Feb 6, 2018

Or maybe instead of it being a simple boolean flag it could actually be a integer priority. The higher the priority (app_status, current file) the less likely it is they will be truncated.

Implemented: app_status, current file and lintlogo have a priority of 2, the default is 1 and for filelist all files other than the current one have a priority of 0.
Within the same priority the "truncate" setting is still honored, e.g. removing/truncating components starts either from the left or from the right.

@Consolatis
Copy link
Contributor Author

Consolatis commented Feb 10, 2018

It would be nice to somehow limit the file list length as it's probably potentially the longest item

This would actually be a nice feature for the filelist module, regardless of the truncating issue.
Not based on the actual length of the entry but the count of files. Some config which says only show the currently active file + the two following files and an additional "file" which just states [$x more files] or something along those lines.

Implemented: 2 config options to configure this:

  • liststyle:
    • active_front (active file on front, old sublemon behavior)
    • active_center (active file in the middle, if filecount is even hide last file to make it odd)
    • linear (if possible start from index 0)
  • limit (show up to $limit entries only, limit <= 0 disables the limit completely)

I have set the default to liststyle: linear and limit: 5.
This tries to start the filelist at index 0 but may start with a higher index if the current file would otherwise not be visible based on limit.
The default limit of 5 should be fine for most cases but may be tweaked by the user.
Obviously the default settings can be changed to whatever @richrd prefers.

Regardless of any of the options, if the screen is too small to show all of the files based on this options the statusbar truncating will still kick in and may remove components on its own. This is completely independent from the filelist module.

@richrd
Copy link
Owner

richrd commented Feb 25, 2018

Hey thanks! The default behavior and colors now look really nice. Way fancier than what I had implemented.

@richrd
Copy link
Owner

richrd commented Feb 26, 2018

A few comments that I thought of when testing this:

  • The logging could probably be shown as DEBUG instead of INFO. That way they won't be printed by default as INFO is the default level to show
  • The Cursors status at the bottom could probably have a colon before the number, similarly as Lint has
  • Would be nice to have the 'battery' status at the top right before the clock, and after the fill

Overall I think the implementation is solid!

@Consolatis
Copy link
Contributor Author

The logging could probably be shown as DEBUG instead of INFO. That way they won't be printed by default as INFO is the default level to show

I think I got all the spammy ones.

The Cursors status at the bottom could probably have a colon before the number, similarly as Lint has

Renamed the old StatusComponent (which only shows the raw number) to "cursor_count" and added a new one called "cursors" which additionally creates the "Cursors: " prefix inside the component. Default top bar is changed to use the new cursors component.

Would be nice to have the 'battery' status at the top right before the clock, and after the fill

added

There are still quite some FIXME's and TODO's inside the code and also some other outstanding issues (as listed in first post) with this PR.
I am not sure when I can dig into those but if you see other issues feel free to point them out :)

@Consolatis Consolatis force-pushed the feature/status_components branch 2 times, most recently from 761d1b7 to 680826b Compare March 8, 2018 09:22
@Consolatis Consolatis force-pushed the feature/status_components branch from 680826b to 03cb939 Compare October 8, 2021 08:15
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.

2 participants