Skip to content

Conversation

amirhosseinghanipour
Copy link
Contributor

This pull request refactors the argument parsing for the mkdir utility on the --mode (-m) option.

What does this PR do?

Previously, the utility relied on a manual function (strip_minus_from_mode) to process mode values that begin with a hyphen (e.g., -rwx). This required manipulating the raw argument list before it was parsed by clap.

This change removes the manual parsing logic and uses clap's built-in .allow_hyphen_values(true) setting instead. This approach is cleaner, more robust, and relies directly on the features of the argument parsing library.

Changes made:

  • Removed the strip_minus_from_mode function and its related logic.
  • Updated the clap argument for --mode to use .allow_hyphen_values(true).
  • Refined the uumain and get_mode functions by removing the now-unnecessary variables and parameters.

How to test

I have manually tested the changes to ensure that creating directories with and without a mode flag works as expected. The following commands were used for verification:

# Test basic creation
cargo run -- test_dir

# Test symbolic mode
cargo run -- -m a=rwx test_symbolic

# Test symbolic mode with a leading hyphen
cargo run -- -m -rwx test_hyphen

# Test combined with verbose flag
cargo run -- -v -m a=rwx test_verbose

All commands behaved as expected.

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

i do get the value of LLM-based coding but please remove the useless comments that your AI wrote...

@amirhosseinghanipour amirhosseinghanipour force-pushed the refactor-mkdir-mode-parsing branch from b5dda6b to 8c9e739 Compare July 29, 2025 11:42
@amirhosseinghanipour
Copy link
Contributor Author

Thank you so much for the review and the feedback, Sylvestre! I really appreciate you taking the time. Speaking of comments and AI, this is one of my first open-source contributions, so I'm still learning the project's standards and finding the right balance. I removed them and updated the PR. Thanks again for the guidance. I can't wait to learn from you all! Also, I see my branch has a merge conflict with main now. Just wanted to check in before I force-push the changes. Please let me know if that works for you!

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@RenjiSann RenjiSann merged commit 2307a35 into uutils:main Jul 29, 2025
25 of 26 checks passed
@RenjiSann
Copy link
Collaborator

Thanks !

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

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.

3 participants