Skip to content

Add max-depth flag and set|unset_max_depth internals #991

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

Merged
merged 5 commits into from
Mar 25, 2025

Conversation

mcky
Copy link
Contributor

@mcky mcky commented Mar 21, 2025

As discussed in #843 this adds the flag --max-depth=N and the internals/verbs :set_max_depth N and :unset_max_depth

Demo video of the new functionality:

CleanShot.2025-03-21.at.18.59.23.mp4

I'm a rust novice and I'm not precious about this code at all, if I've gone in the wrong direction I'm happy to hop on miaou and discuss.

Notes

  • The depth passed doesn't include the current folder, which I think is more intuitive
  • --max-depth=0 is allowed but basically only shows the current directory, so is useless. If we want to validate this, I wasn't sure where to do so
  • I couldn't find any similar test suites to extend, so have only manually tested it

Questions

  • Should I add this to Conf?
  • What should the short flag be? -d is taken and -m doesn't feel ideal
  • Shall I add a verb shortcut? md / umd?

As an aside I noticed there's a bit of mixed indentation in panel_state.rs and running cargo fmt creates a lot of changes (that'll teach me to run it without staging my changes..), so I've tried to format it as consistently as I can

@Canop
Copy link
Owner

Canop commented Mar 22, 2025

Should I add this to Conf?

Not for now.

What should the short flag be? -d is taken and -m doesn't feel ideal

Let's keep it without short flag.

Shall I add a verb shortcut? md / umd?

md is make directory right now. We can keep it without verb shortcut, as user can add it if desired.

@mcky
Copy link
Contributor Author

mcky commented Mar 25, 2025

@Canop Thanks for the feedback and patience, I've moved the check to gather_lines, swapped for a u16 and dropped the shorthand flag. Left the Cow change as yeah I'm not confident in trying to quickly swap it out here

@Canop
Copy link
Owner

Canop commented Mar 25, 2025

It seems to work very well. I'll merge that.

@Canop Canop merged commit 4d10cb4 into Canop:main Mar 25, 2025
1 check passed
@mcky mcky deleted the max-depth-flag branch March 25, 2025 17:16
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.

2 participants