nvim-tree.lua icon indicating copy to clipboard operation
nvim-tree.lua copied to clipboard

Refactor: attributes modules & line builder

Open chomosuke opened this issue 2 years ago • 5 comments

As discussed in https://github.com/nvim-tree/nvim-tree.lua/pull/1871#discussion_r1059823270

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.

chomosuke avatar Jan 06 '23 03:01 chomosuke

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 avatar Jan 07 '23 03:01 alex-courtis

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.

alex-courtis avatar Jan 07 '23 03:01 alex-courtis

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.

chomosuke avatar Jan 07 '23 10:01 chomosuke

Love your work!

alex-courtis avatar Jan 08 '23 01:01 alex-courtis

#2415 addresses some points.

alex-courtis avatar Dec 03 '23 03:12 alex-courtis