Skip to content

feat: configurable indent marker level #1570

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

Closed
wants to merge 1 commit into from

Conversation

sand4rt
Copy link
Contributor

@sand4rt sand4rt commented Sep 28, 2024

closes #1512

@sand4rt sand4rt force-pushed the indent-level branch 2 times, most recently from 7f67024 to 86a78c5 Compare September 28, 2024 20:30
@sand4rt sand4rt changed the title feat: configurable indent level feat: configurable marker indent level Sep 29, 2024
@sand4rt sand4rt changed the title feat: configurable marker indent level feat: configurable indent marker level Sep 29, 2024
Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is questionable. I don't know for sure, but other code may assume that level 0 means the root node and this could be more than just a visual tweak.

There is also a padding option on the container which may be more appropriate. Either way, see: #1606

@sand4rt
Copy link
Contributor Author

sand4rt commented Feb 21, 2025

@pynappo could this be merged? i use this for a while now without issues

@pynappo
Copy link
Collaborator

pynappo commented Feb 22, 2025

The option should be renamed to something more descriptive like guide_start_level. also, add it to defaults.lua

also in the original issue you mention that you want to use this to add indent guides for levels 0/1. if that's the case, then levels 0/1 should be better handled to have no padding on the left, similar to level 2 and up

0 should have no padding on the left here:

image

1 should have no padding on the left here (the guide should line up with the folder icon of the root node):
image

2 looks fine:

image

@and-rs
Copy link

and-rs commented Feb 26, 2025

I would like this merged too. Any way in which I could contribute?

Copy link

@and-rs and-rs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this is good

@sand4rt
Copy link
Contributor Author

sand4rt commented Feb 26, 2025

I would like this merged too. Any way in which I could contribute?

Gladly! Feel free to create a PR against this branch or rebase it and open a new PR, then ill close this one.

I'd say this is good

There are still some issues in there. This is described in: #1570 (comment)

@and-rs
Copy link

and-rs commented Feb 27, 2025

alrightt of course, there were some issue with the implementation and I also have some questions about how we should handle the end result.

  1. Do we really want the indentation marker to start from the node 0? Or is it fine at the node 1? Because I honestly think that node 0 is just not useful at all. I wanted to implement this because if I hid the root node with hide_root_node = true, the first nodes did not have a marker. Take a look at this:
combined
  1. if there are hidden files in some directory after a node with markers, the marker doesn't get rendered, and I am just not sure how to deal with that.
  • This is another example on how it looks with guide_level_start = 0 and with hide_root_node = false.
Screenshot 2025-02-26 at 8 27 30 PM

@sand4rt
Copy link
Contributor Author

sand4rt commented Feb 27, 2025

Do we really want the indentation marker to start from the node 0? / This is another example on how it looks with guide_level_start = 0

Yes, this is what the issue is about. I like how it looks. Perhaps this is fixable with an if opts.hide_root_node then ..

there are hidden files in some directory after a node with markers

Not sure either, could send what you already have? Maybe someone could help out with it?

@and-rs
Copy link

and-rs commented Feb 27, 2025

Alright if you like the node 0 implementation, I'd say we leave it like that, I don't even think a conditional would be necessary, and it would allow for more customization, anyone could decide if they want the indent marker from the node 0 with or without the root node hidden. I also recommend calling the option, marker_start_level just like the other options related to the indent marker.

I was wondering if I could commit directly to this PR branch, or if another PR is needed, I appreciate the guidance.

@sand4rt
Copy link
Contributor Author

sand4rt commented Feb 27, 2025

I was wondering if I could commit directly to this PR branch, or if another PR is needed

both is fine

@and-rs
Copy link

and-rs commented Feb 28, 2025

alright, I just made a new PR #1705 with the changes we discussed here. feel free to test them

@sand4rt
Copy link
Contributor Author

sand4rt commented Mar 2, 2025

closing in favour of: #1705

@sand4rt sand4rt closed this Mar 2, 2025
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.

FEATURE: indent marker level
4 participants