Update arg parsing to clap v3 #1717

tranzystorekk · 2022-01-03T18:53:20Z

IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.

The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.

Switches arg parsing to the modern derive style via the newly released clap 3.0.0

Important/to discuss:

Now the help is colored by default
The base_path/base_url parameters for the check subcommand were read from ArgMatches but never defined in the clap app, so I set them to None for now

Sanity check:

Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

(Delete or ignore this section for documentation changes)

Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

Have you created/updated the relevant documentation page(s)?

Keats

Thanks for the PR!

src/cli.rs

Keats · 2022-01-04T10:14:59Z

src/main.rs

+    let root_dir = cli
+        .root
+        .canonicalize()
+        .unwrap_or_else(|_| panic!("Cannot find root directory: {}", cli.root.display()));


that's not handling the . case?

Here cli.root already has the . value if it wasn't specified on the command line, so we handle only the canonicalize operation result, which I think should work for .

I'm not entirely sure it does work on all platforms, I'd like to be certain before changing it

I built a small program to verify that canonicalizing . works on windows, but if we wanted to be extra careful, there's the normpath crate that seems to be saner (also personally confirmed that it works).

Let's keep like you have for now, we'll see if it causes an issue

src/main.rs

Keats · 2022-01-04T10:18:56Z

src/cli.rs

+        /// Include drafts when loading the site
+        #[clap(long)]
+        drafts: bool,
+    },


can we be explicit with all the default values as well? eg set false for drafts etc. I guess Subcommand relies on default but I'd like to be able to see the full set of option values at a glance

I've looked at the documentation, but especially for bool flags this is neither elegant to write nor widely accepted style to specify the default explicitly.

oh :(
Just passing true/false to default doesn't work? It's kind of weird that non bool gets to have their default value in the struct but not bool since there are only 2 possible values...

Did a quick check and clap protests with a "default value is meaningless for bool" error.

I'm guessing the rationale is that bool flags are simple on-off switches and don't require any special handling.

Keats · 2022-01-10T11:56:24Z

Cargo.toml

@@ -13,14 +13,15 @@ keywords = ["static", "site", "generator", "blog"]
 include = ["src/**/*", "LICENSE", "README.md"]

 [build-dependencies]
-clap = "2"
+clap = "3"
+clap_complete = "3"


what does this do and where is it used?

In v3 the completion functionality has been split off to the separate clap_complete package, I noticed that there is some completion code in build.rs that is commented out, so admittedly this is a bit preemptive.

I guess we could instead add it in a separate PR focused on bringing back completion

tranzystorekk · 2022-01-10T12:01:46Z

The remaining issue is that clap v3 uses the include_str! macro to add README.md contents to its docs, which effectively makes it MSRV 1.54, while linux-pinned uses 1.52

Keats · 2022-01-10T12:06:45Z

Bumping the MSRV is not an issue but can that feature be disabled as it wouldn't be used anyway?

tranzystorekk · 2022-01-10T12:18:24Z

but can that feature be disabled as it wouldn't be used anyway?

I'm afraid building docs for dependencies can only be disabled in cargo doc, and the error occurs in cargo build in general.

Keats · 2022-01-10T12:56:04Z

I'm afraid building docs for dependencies can only be disabled in cargo doc, and the error occurs in cargo build in general.

Oh I see, I thought clap was somehow pulling the readme in the built cli. This makes more sense.

Keats · 2022-01-23T13:45:52Z

Can you resolve the conflicts? Or i'll do it later today

Keats · 2022-01-23T19:21:44Z

Thanks!

* Update arg parsing to clap v3

Keats requested changes Jan 4, 2022

View reviewed changes

Keats approved these changes Jan 10, 2022

View reviewed changes

Keats reviewed Jan 10, 2022

View reviewed changes

Keats force-pushed the next branch 2 times, most recently from 189c37d to 6238b36 Compare January 23, 2022 13:38

tranzystorekk changed the base branch from next to master January 23, 2022 14:56

tranzystorekk changed the base branch from master to next January 23, 2022 15:00

tranzystorekk added 4 commits January 23, 2022 16:02

Update arg parsing to clap v3

2311340

Add small flag definition adjustments

2a12636

Handle simple errors with if let

134c7e6

Update pinned Rust version to 1.54

0ee487d

tranzystorekk force-pushed the clap-v3 branch from 4dcd6b6 to 0ee487d Compare January 23, 2022 15:04

Keats approved these changes Jan 23, 2022

View reviewed changes

Keats merged commit c9b2d35 into getzola:next Jan 23, 2022

tranzystorekk deleted the clap-v3 branch January 23, 2022 19:40

Keats pushed a commit that referenced this pull request Feb 14, 2022

Update arg parsing to clap v3 (#1717)

0f23a40

* Update arg parsing to clap v3

mwcz pushed a commit to mwcz/zola that referenced this pull request Mar 7, 2022

Update arg parsing to clap v3 (getzola#1717)

701c178

* Update arg parsing to clap v3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update arg parsing to clap v3 #1717

Update arg parsing to clap v3 #1717

tranzystorekk commented Jan 3, 2022 •

edited

Loading

Keats left a comment

Keats Jan 4, 2022

tranzystorekk Jan 4, 2022

Keats Jan 4, 2022

tranzystorekk Jan 9, 2022

Keats Jan 10, 2022

Keats Jan 4, 2022

tranzystorekk Jan 4, 2022 •

edited

Loading

Keats Jan 4, 2022

tranzystorekk Jan 4, 2022 •

edited

Loading

Keats Jan 10, 2022

tranzystorekk Jan 10, 2022

tranzystorekk commented Jan 10, 2022 •

edited

Loading

Keats commented Jan 10, 2022

tranzystorekk commented Jan 10, 2022

Keats commented Jan 10, 2022

Keats commented Jan 23, 2022

Keats commented Jan 23, 2022

Update arg parsing to clap v3 #1717

Update arg parsing to clap v3 #1717

Conversation

tranzystorekk commented Jan 3, 2022 • edited Loading

Code changes

Keats left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tranzystorekk Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tranzystorekk Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tranzystorekk commented Jan 10, 2022 • edited Loading

Keats commented Jan 10, 2022

tranzystorekk commented Jan 10, 2022

Keats commented Jan 10, 2022

Keats commented Jan 23, 2022

Keats commented Jan 23, 2022

tranzystorekk commented Jan 3, 2022 •

edited

Loading

tranzystorekk Jan 4, 2022 •

edited

Loading

tranzystorekk Jan 4, 2022 •

edited

Loading

tranzystorekk commented Jan 10, 2022 •

edited

Loading