feat(#3113): add renderer.icons.folder_arrow_padding

[smchunn]

resolves #3113

default: opts.renderer.icons.folder_arrow_padding = " "

[alex-courtis]

You legend @smchunn

I'll get to a review and test over the weekend.

[alex-courtis]

Looks like you'll need to update help: https://github.com/nvim-tree/nvim-tree.lua/blob/master/CONTRIBUTING.md#documentation

[alex-courtis]

Working nicely.

• add new option table and legacy fallback

Full test matrix for config items that affect padding:

renderer.indent_width

renderer.icons.padding
renderer.icons.folder_arrow_padding

renderer.icons.show.folder_arrow

renderer.indent_markers.enable
renderer.indent_markers.inline_arrows
renderer.indent_markers.icons ...

That's not practical. Testing just those that affect get_arrows.
Directory and not directory.
Uncomment each test as you go

renderer.icons.glyphs.folder.arrow_closed = ">"
renderer.icons.glyphs.folder.arrow_open = "|"
renderer.icons.glyphs.folder.default = "c"
renderer.icons.glyphs.folder.open = "o"

-- renderer.icons.show.folder_arrow = false

-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = false
-- renderer.icons.folder_arrow_padding = "-"

-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = false
-- renderer.icons.folder_arrow_padding = ""

-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = true
-- renderer.icons.folder_arrow_padding = "-"

-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = false
-- renderer.icons.folder_arrow_padding = "----"

-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = true
-- renderer.icons.folder_arrow_padding = "----"

-- renderer.icons.glyphs.folder.arrow_closed = ""
-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = true
-- renderer.icons.folder_arrow_padding = "-"

-- renderer.icons.glyphs.folder.arrow_open = ""
-- renderer.icons.show.folder_arrow = true
-- renderer.indent_markers.enable = true
-- renderer.icons.folder_arrow_padding = "-"

[alex-courtis]

This is looking really good.

Let's refactor the options now:

  renderer = {
    icons = {
      padding = {
        icon = " ",
        folder_arrow =  " ",
      },
  },

We do this via legacy.lua and document it in :help nvim-tree-legacy-opts.

renderer.icons.webdev_colors looks like a good one to emulate - it moves a higher level option into a new subtable.

[alex-courtis]

Apologies, I steered you down a very incorrect path with the legacy conversion. Using the util is not possible as the source and destination are the same.

In the interests of speed I've pushed up a fix to your branch, please tell me if you don't prefer that.

Test cases passed:

empty config

renderer.icons.padding = nil

renderer.icons.padding = {}

renderer.icons.padding = "i"

• fails validation
  renderer.icons.padding = 99

renderer.icons.padding.icon = "i"
renderer.icons.padding.folder_arrow = "a"

• fails validation
  renderer.icons.padding.icon = 99
  renderer.icons.padding.folder_arrow = "a"

• fails validation
  renderer.icons.padding.icon = "i"
  renderer.icons.padding.folder_arrow = 99

[alex-courtis]

• Please re-test following my changes

[gegoune]

Just nitpicking: I am not huge fan of folder_arrow name. Could we come up with something better? (_arrow part is what doesn't sit right with me.) Perhaps something like folder_indicator, or else, not sure. There, obviously is a precedent as we have *arrow* all over other folder and symlink related options. Perhaps something for future breaking change?

[alex-courtis]

Just nitpicking: I am not huge fan of folder_arrow name. Could we come up with something better? (_arrow part is what doesn't sit right with me.) Perhaps something like folder_indicator, or else, not sure. There, obviously is a precedent as we have *arrow* all over other folder and symlink related options. Perhaps something for future breaking change?

Thank you! Yes please, I didn't see the forest for the trees.

[smchunn]

• fails validation
  renderer.icons.padding.icon = "i"
  renderer.icons.padding.folder_arrow = "a"

@alex-courtis Is this a mistake?

All other test cases yielded the same results. I am good to go with your changes. Realized I forgot to update the actual padding logic when refactoring. Oops. Thanks for taking care of that one.

Also not a fan of the name, but I do think folder_arrow makes sense logically as it is the padding for renderer.icons.glyphs.folder.arrow_open/closed

[alex-courtis]

• fails validation
  renderer.icons.padding.icon = "i"
  renderer.icons.padding.folder_arrow = "a"

@alex-courtis Is this a mistake?

Apologies, yes that is. It should be folder_arrow = 99

Oops. Thanks for taking care of that one.

Do not stress; it is fragile, undocumented and annoying to test.

Also not a fan of the name, but I do think folder_arrow makes sense logically as it is the padding for renderer.icons.glyphs.folder.arrow_open/closed

I see. It is arrow everywhere. I didn't look before replying...
A non-breaking change of all the arrow names (similar to root_folder_modifier -> root_folder_label) can be done simply. Another PR @gegoune ?

[alex-courtis]

Many thanks for your work @smchunn ! Merging...

You've brought up a good point @gegoune - there are still some badly named options. We've dealt with several like update_cwd -> sync_root_with_cwd

You've got messy ones like renderer.icons.git_placement -> renderer.icons.placement.git that are still clear.
There are misplaced ones like diagnostics.icons that should be with the others in renderer.icons.glyphs.
Ones like ui.confirm.default_yes are not clear and can be renamed.

Is it time for a review and overhaul of options?

[gegoune]

@alex-courtis Can use #2821 to track it.