Skip to content

feat: Add --sort arg to CLI to sort by path/size/dates#1982

Open
DeflateAwning wants to merge 20 commits intosharkdp:masterfrom
DeflateAwning:feat-sort-arg
Open

feat: Add --sort arg to CLI to sort by path/size/dates#1982
DeflateAwning wants to merge 20 commits intosharkdp:masterfrom
DeflateAwning:feat-sort-arg

Conversation

@DeflateAwning
Copy link
Copy Markdown

Resolves #1875

Copy link
Copy Markdown
Collaborator

@tmccombs tmccombs left a comment

Choose a reason for hiding this comment

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

If a sorting feature is added to fd, it will need to have very minimal impact on flows that don't use sort, and shouldn't significantly add to the complexity and maintenance burden of the project.

This isn't there (yet).

Comment thread src/filter/mod.rs Outdated
Comment thread src/cli.rs Outdated
Comment thread tests/tests.rs
Comment thread src/walk.rs
Comment thread src/walk.rs Outdated
Comment thread src/walk.rs Outdated
Comment thread src/walk.rs Outdated
Comment thread src/walk.rs
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This implementation requires sorting to be handled by every output path. That adds complexity and makes it harder to maintain.

It would be better if there is some way we could avoid needing separate sorting logic for --exec, --exec-batch, and printing.

@DeflateAwning
Copy link
Copy Markdown
Author

Thank you for the review! One or two days and I'll have another version to look over.

@DeflateAwning
Copy link
Copy Markdown
Author

@tmccombs Thanks again for the review/suggestions. Would you mind taking another look? It's getting closer, I think.

Regarding the "one code path" suggestion - I'm open to ideas. I think it's reasonably DRYed right now. The alternative implementation I thought of involves piping the iterator through something like itertools::sorted, but I'm not sure I see how to make that happen, and I'm not convinced it'd be much cleaner.

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.

Add a --sort <path|size|created|modified> option, especially for use with -x <cmd>

2 participants