Skip to content

Refactored ls.rs to fix cargo fmt#2434

Closed
hbina wants to merge 1 commit intouutils:masterfrom
hbina:hbina-ls-refactor-to-fix-fmt
Closed

Refactored ls.rs to fix cargo fmt#2434
hbina wants to merge 1 commit intouutils:masterfrom
hbina:hbina-ls-refactor-to-fix-fmt

Conversation

@hbina
Copy link
Contributor

@hbina hbina commented Jun 19, 2021

cargo fmt have trouble formatting large clap::Arg.
This commit simply splits some stuff up so that the implementation details
can be formatted.

Signed-off-by: Hanif Bin Ariffin hanif.ariffin.4326@gmail.com

`cargo fmt` have trouble formatting `clap::Arg`.
This commit simply splits some stuff up so that the implementation details
can be formatted.

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@sylvestre
Copy link
Contributor

cargo fmt have trouble formatting large clap::Arg.

Do you please explain this a bit more?
Are you sure it isn't this issue rust-lang/rustfmt#3863 ?

Anyway, I don't think we should split the files just to make rustfmt happy?

@tertsdiepraam wdyt?

@tertsdiepraam
Copy link
Collaborator

I've seen in another PR that rustfmt fails if the help message is too long, so maybe we should just break those up. Example:

// From
.help("this message is a very very very long help message that doesn't fit")
// To
.help(
    "this message is a very very very \
    long help message that doesn't fit"
)

Most arguments in ls seem to have this already, but maybe they should be broken off earlier?

@tertsdiepraam
Copy link
Collaborator

I do actually think this should be broken up though, but I would wait for #2414 to get merged and work from there.

@hbina hbina mentioned this pull request Jun 19, 2021
@hbina
Copy link
Contributor Author

hbina commented Jun 19, 2021

Okay, I will try to refactor the constants first. Maybe by making options a file of its own. I am also thinking of grouping the flags and their help/after_help description together.

@hbina hbina closed this Jun 19, 2021
@hbina hbina deleted the hbina-ls-refactor-to-fix-fmt branch June 20, 2021 12:28
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