Skip to content

Refactor: attributes modules & line builder #1889

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

Open
9 tasks
chomosuke opened this issue Jan 6, 2023 · 5 comments
Open
9 tasks

Refactor: attributes modules & line builder #1889

chomosuke opened this issue Jan 6, 2023 · 5 comments
Labels
performance performance enhancement

Comments

@chomosuke
Copy link
Collaborator

chomosuke commented Jan 6, 2023

As discussed in #1871 (comment)

Background

nvim-tree is a very customizable plugin and it provide options to display certain attributes of files and directories.

Currently, 4 such attributes are implemented: git, diagnostics, modified and opened. However, they're all implemented separately, and as a result, have duplicated logic between them and lack feature parity (e.g. can only display diagnostics in signcolumn, can't see if file in a directory are opened without opening the directory).

Further more, it is more difficult that necessary to implement new attributes as there's no existing framework to allow such implementation without an understanding of how the renderer works. Many of the attributes' implementation are also tightly coupled with the rest of nvim-tree making maintenance more difficult.

Proposed solution

We split nvim-tree into three distinct part: core, attributes modules & line builder.

Git, diagnostics, modified & opened becomes attributes modules. They interact with core to attach attributes to paths (i.e. directories & files). Core can propagate attributes to parents / children at the attributes module's request.

Core calls line builder with all the path's attributes + other info like name, depth. Line builder returns everything the core needs to display including padding, signcolumn, highlights etc.

Resulting improvements

Direct improvements

  • Maintenance and the implementation of new attributes will be easier as attributes modules will be decoupled from the rest of nvim-tree.
  • User will be allowed to add their own attributes modules. If the modules is deemed useful enough we can add them as one of nvim-tree's build in modules.
  • User will be allowed to override the line builder.

Potential improvements

  • User can be allowed to filter based on arbitrary attributes.
  • Core can be made to only call line builder for paths that have their attributes changed instead of having to do a full reload, thus improving performance.

Decision to be made

  • Structure of the new config:
    • Do we break?
      • We can have more sensible default.
      • We can reorganize configurations, making configuration of all attributes modules more consistent with each other & making it better suited for the current architecture.
      • We can remove / change some of the API to better suit the current architecture, some of the API/option makes less sense with the refactoring such a custom_filter.

Steps

  • Expose API for attribute module to communicate with core.
  • Refector existing attributes to use the new API.
    • opened
    • modified
    • diagnostics
    • git
  • Decouple line builder from the rest of nvim-tree.
  • Restructure options (optional).
  • Add option to let user add their own attributes modules & override line builder.
@alex-courtis
Copy link
Member

This is fantastic, thank you for taking the initiative.

We split nvim-tree into three distinct part: core, attributes modules & line builder.

attributes are the bulk of the operations and are cheap i.e. no file system operations. Isolating and extracting these will result in great performance improvements and greater readability/reasonability.

Do we break?

We don't need to make any decisions now. Start things off and we will see if breaking is even necessary.

Expose API for attribute module to communicate with core.

Internal only for now please. Let's not publish anything until we're sure of our final state.

@alex-courtis
Copy link
Member

I might suggest we start with a high value / low complexity one like opened.

The API can evolve as we need it e.g. propagation.

@chomosuke
Copy link
Collaborator Author

chomosuke commented Jan 7, 2023

We don't need to make any decisions now. Start things off and we will see if breaking is even necessary.

I don't think breaking is necessary. I just feel like we'd benefit from being able to make breaking changes (i.e. have some sort of versioning). I was thinking if we can have proper versioning before we finish this refactoring, we can put into the next version.

Internal only for now please. Let's not publish anything until we're sure of our final state.

Sure thing, exposing everything to the user will be our final step.
I've actually already draft up one possible API we can use: https://github.com/nvim-tree/nvim-tree.lua/blob/opened-file-feature-parity/note.md
But I'll keep everything internal for now.

I might suggest we start with a high value / low complexity one like opened.

Already working on it :) https://github.com/nvim-tree/nvim-tree.lua/tree/opened-file-feature-parity, I'm a bit busy recently so it might take me a while though.

@alex-courtis
Copy link
Member

Love your work!

@alex-courtis alex-courtis added the performance performance enhancement label Jan 9, 2023
@alex-courtis
Copy link
Member

#2415 addresses some points.

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

No branches or pull requests

2 participants