Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed --files argument to sort alphabetically #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Alexe1289
Copy link

@Alexe1289 Alexe1289 commented Jan 9, 2024

Now if --files argument is found it will sort by default alphabetically the name of the files. If --sort argument is also provided (for example tokei --files --sort blanks), the sorting for files will be ignored and it will only sort by given argument.
Before:
image

After:
image

Should fix XAMPPRocky#1053

Copy link

@alexandruradovici alexandruradovici left a comment

Choose a reason for hiding this comment

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

Did you run cargo fmt?

src/cli_utils.rs Outdated
@@ -337,14 +337,16 @@ impl<W: Write> Printer<W> {

if self.list_files {
self.print_subrow()?;

let mut reports: Vec<_> = language.reports.clone();

Choose a reason for hiding this comment

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

Why clone this?

Copy link
Author

@Alexe1289 Alexe1289 Jan 9, 2024

Choose a reason for hiding this comment

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

Because "language" is borrowed and sort_by function requires mutable access.

Copy link

@alexandruradovici alexandruradovici Jan 9, 2024

Choose a reason for hiding this comment

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

You could create a new Vec<&R> from language.reports which stores the data type R and sort that one. You can use .map and .collect.

Copy link
Author

@Alexe1289 Alexe1289 Jan 9, 2024

Choose a reason for hiding this comment

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

I don't know if this is how I should do it, if I try like this to get a Vec , it says that the Copy Trait is not implemented.

image

And I can't use a Vec<&Report> because I would have to change the code below to accept &Report
let (a, b): (Vec<_>, Vec<_>) = reports .iter() .partition(|r| r.stats.blobs.is_empty());

Choose a reason for hiding this comment

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

Please use

post code here

to post code instead of pictures.

Choose a reason for hiding this comment

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

You want to get a Vec<&Report>, instead you are trying to take ownership here.

Copy link
Author

Choose a reason for hiding this comment

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

Pushed a commit now with the requested change. I will run fmt on it after feedback,

@Alexe1289
Copy link
Author

Alexe1289 commented Jan 9, 2024

Did you run cargo fmt?

No, I didn't.
How to run it? I get an error if I try :

image

@alexandruradovici
Copy link

Did you run cargo fmt?

No, I didn't. How to run it? I get an error if I try :

image

You might need to install the fmt rust component.

@alexandruradovici
Copy link

Looks good, please send it to upstream and place a link to it here.

@Alexe1289
Copy link
Author

Alexe1289 commented Jan 18, 2024

XAMPPRocky#1059

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How are the filenames listed by the '--files' option ordered?
2 participants