[Feature request] Equivalent of Ranger's `set draw_borders separators` setting

[wincent]

In Ranger, you can get it to draw separators only between the columns.

lf has the comparable set drawbox setting, but it draws borders around all edges, including the top and bottom ones.

Could we please have a setting to draw only separators? That would free up two lines (one at top, one at bottom) and two columns (one at far left, one at far right), which is useful given limited screen real estate.

[gokcehan]

@wincent That sounds quite reasonable. However, we should probably figure out a way to do this without a breaking change if possible. It is unfortunate that I didn't realize ranger's draw_borders is not a boolean option when we added drawbox. From what I understand, ranger has converted the option from a boolean to a string at some point without a breaking change. I don't think we can do that in lf since we still support set booloption syntax in addition to set booloption true. So we probably need a separate option for this (e.g. maybe drawsep or boxstyle). Ideas are welcome.

Beyond the details of the option, I don't expect the implementation of this feature to be much difficult. So if anyone wants to give this a try, feel free to do so. I don't have much free time myself these days.

[wincent]

@gokcehan I sent a PR with a possible implementation here. Lightly tested it and it seems to work.

From what I understand, ranger has converted the option from a boolean to a string at some point without a breaking change. I don't think we can do that in lf since we still support set booloption syntax in addition to set booloption true.

I'm not sure if this is a problem. set drawbox is equivalent to set drawbox "", which we treat she same as set drawbox true/set drawbox both. So I think it just works? 🤷🏻

[gokcehan]

@wincent Thank you very much for the PR. That approach could work, but isn't it a little confusing that set drawbox and set drawbox "" are equivalent to set drawbox "all" and not set drawbox "none"? I'm also not sure about "all" and "both" as option values as they can limit any possible future additions. For example if someone wants to add an option to extend the borders to top and the bottom lines, it will either change the default (i.e. "all") or it will lose the clarity (i.e. "both"). Since the option name drawbox is also different from ranger, it may not be worth to be compatible with ranger. I think our convention so far was to use []string as the values for such options (e.g. info or preserve). One possibility that I haven't explicitly mentioned is that maybe we can add a new option and also deprecate the old one. As much as I dislike breaking changes, I'm not a fan of inconsistencies either. Since this is mostly a cosmetic feature, maybe it is somehow acceptable to make a breaking change. It can also help us get rid of the name "box" as it is a little strange to use this name for only separators.

[wincent]

Ok, so how about this:

1. Mark drawbox as deprecated, but translate it internally to the new setting¹. That way we can defer the actual breaking change of removing drawbox as long as we want.
2. Add a new setting called drawborders of type string[] that would take values top, bottom, left, right, inner.

With the new setting, you could get equivalent of the ranger settings with:

Ranger value	lf drawborders value
outline	top right bottom left
separators	inner
both/true	top right bottom left inner
none/false	(empty string)

People would be able to use other combinations that Ranger doesn't support either, if they wanted 🤷🏻 — eg. top bottom or top bottom inner etc.

We could optionally add all as a shortcut for saying top right bottom left inner.

Obviously, all of those names are just ideas and up for discussion. If you have a clear idea of what names you'd want, I can take a stab at rolling an updated PR.

Footnotes

1. So, set drawbox would be equivalent to set drawborders top right bottom left inner, or set drawborders all, if we add all as well. ↩

[gokcehan]

@wincent That sounds like a good plan to me. We can also call the new option simply border or borders if you like. By the way, when I said extending the borders to top and bottom, I meant something like this:

┌────────────────────────────────────────────────┐
│gokcehan@this:/tmp/asd/foo/bar                  │
├────────────┬───────────────────────┬───────────┤
│  foo       │  bar                  │  file1    │
│            │  baz                  │  file2    │
│            │                       │  file3    │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
│            │                       │           │
├────────────┴───────────────────────┴───────────┤
│drwxr-xr-x 2 gokcehan gokcehan ...           1/2│
└────────────────────────────────────────────────┘

I think maybe midnight commander has such a layout for its bottom line so I thought maybe someone can request it in the future. In any case, this is not something that needs to be implemented in this patch. I just thought it could be nice to leave it as a possibility for the future. Apart from that, I don't see much benefit in having fine grained controls at the moment, so you might as well just leave the patch as it is with only two cases (e.g. inner and outer). Fine grained controls can still be added later on if required similar to how things works with i3 gaps:

Outer gaps can be configured for each side individually with the top, left, bottom and right directive. horizontal and vertical are shortcuts for left/right and top/bottom, respectively.

If we decide to add all as an option value, maybe we should discuss doing something similar for all applicable []string options. We should also explicitly mention in the documentation that the included values are subject to changes in the future. If you ask me, all should not be required if we have a value for outer, so we should probably leave this decision for later.

I think it is a good idea to leave this issue/PR open for a while for possible feedback from others. You may or may not want to update your patch in the meantime as you wish. Note, I will not be available next week, but we have other contributors who can merge the patch when I'm not around. I'm generally okay with whatever decision comes out from this discussion.

[joelim-work]

I took a look at the patch as well, some thoughts from me:

• I think it's fine to keep this as a string value option for now without fine-grained controls as suggested by @gokcehan. Though in addition to the inner and outer cases, we would need to support enabling both (e.g. both), or none of them (e.g. blank string). I am also not sure how to phrase the case where borders are extended to cover the prompt at the top and the status line at the bottom, but we can cross that bridge later.

• I agree it's better to introduce a new option rather than change the existing drawbox option. It's probably not worth supporting drawbox anymore, and just change the code to echoerr a deprecation message whenever a user tries to set that option.

• I think it's important to calculate the window sizes as precisely as possible. Consider the code for the getWidths function:

  if gOpts.drawbox == "both" || gOpts.drawbox == "separators" {
  	wtot -= (wlen + 1)
  }

  The original intention is to reserve spaces for drawing both the outer and inner (separator) walls (hence wlen + 1), but if only the separators are required, then there is no need to reserve space for the outer walls, and the value should be wlen - 1.

• In the original code, if drawbox is disabled, then no horizontal space is reserved but when drawing the panes the space is artificially shrunk by one space to maintain a visual separation between panes. To me this does feel like a hack, and maybe it's better to unconditionally reserve a space between panes so that no code is required for the printDir function. Then, the only difference between setting inner and is whether the separators are drawn or not, and that the columns should not move when changing this option.