Skip to content
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

Sort by path before outputting directory diff so that the order is always the same #593

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

milandamen
Copy link
Contributor

Previously, when doing a directory diff, the order in which the results were displayed was random, depending on thread execution and time it took for a file's diff.

This commit makes it so that the results are always shown in the same order.

It does this by diffing in parallel, then collecting the result, and then sorting it before printing it to the terminal.

This is the first time I have programmed in Rust. Any feedback is welcome.
I can also make a test for this, but as the order is random, the test might still pass even in the old situation.

I have run cargo fmt and cargo test and have tested it manually in a different codebase.

@Wilfred
Copy link
Owner

Wilfred commented Nov 18, 2023

This can make the UI way slower if there's any files with a lot of changes.

I'm not opposed to this feature, but could you put it behind a flag?

@milandamen
Copy link
Contributor Author

Thank you for the quick reply, and no problem. I will put it behind a feature flag. Do you want this functionality enabled or disabled by default?

@Wilfred
Copy link
Owner

Wilfred commented Nov 18, 2023

Disabled by default.

…rting paths when diffing directory (default disabled)
@milandamen
Copy link
Contributor Author

@Wilfred I have made the changes you requested.

@Wilfred Wilfred merged commit a8d6253 into Wilfred:master Nov 20, 2023
13 checks passed
@Wilfred
Copy link
Owner

Wilfred commented Nov 20, 2023

Thanks :)

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