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

Move to clap_derive so CLI arguments are typed #75

Closed
wants to merge 3 commits into from

Conversation

AndreasBackx
Copy link
Member

@AndreasBackx AndreasBackx commented Nov 7, 2023

Move to clap_derive so CLI arguments are typed

  • Renames clap.rs to cli.rs because otherwise there's confusion between the clap crate and local module.
  • Cli struct added that almost identically represents the current state of the CLI with no logical changes.

--help Comparison

Before: https://gist.github.com/AndreasBackx/5945b366e989159f4669e7ba30c13239

After: https://gist.github.com/AndreasBackx/8929c8bde080eac0cafd33128210b0cc

Diff (ignoring whitespace changes due to table alignment):

1c1
< Screenshot tool for compositors implementing zwlr_screencopy_v1.
---
> Screenshot tool for wlroots based compositors implementing the zwlr_screencopy_v1 protocol.
9d8
<   -c, --cursor                      Enable cursor in screenshots
10a10
>   -c, --cursor                      Enable cursor in screenshots
12c12
<   -l, --listoutputs                 List all valid outputs
---
>   -l, --list-outputs                List all valid outputs
14c14
<       --chooseoutput                Present a fuzzy selector for outputs
---
>       --choose-output               Present a fuzzy selector for outputs
16a17
>

You can see that the only changes are:

This seems like a tiny improvement? I plan to make further changes later, but I want to keep PRs separate.


Stack created with Sapling. Best reviewed with ReviewStack.

Shinyzenith
Shinyzenith previously approved these changes Nov 9, 2023
Decodetalkers and others added 3 commits February 2, 2024 19:18
* Renames `clap.rs` to `cli.rs` because otherwise there's confusion between the `clap` crate and local module.
* `Cli` struct added that almost identically represents the current state of the CLI with no logical changes.

---

## `--help` Comparison

Before: https://gist.github.com/AndreasBackx/5945b366e989159f4669e7ba30c13239

After:  https://gist.github.com/AndreasBackx/8929c8bde080eac0cafd33128210b0cc

Diff (ignoring whitespace changes due to table alignment):
```diff
1c1
< Screenshot tool for compositors implementing zwlr_screencopy_v1.
---
> Screenshot tool for wlroots based compositors implementing the zwlr_screencopy_v1 protocol.
9d8
<   -c, --cursor                      Enable cursor in screenshots
10a10
>   -c, --cursor                      Enable cursor in screenshots
12c12
<   -l, --listoutputs                 List all valid outputs
---
>   -l, --list-outputs                List all valid outputs
14c14
<       --chooseoutput                Present a fuzzy selector for outputs
---
>       --choose-output               Present a fuzzy selector for outputs
16a17
>
```

You can see that the only changes are:
- About is longer, this is now using the value from Cargo.toml instead of a duplicate text that was shorter.
- Some have a dash where the English words would have a space, e.g: "list-outputs" instead of "listoutputs". I've also made the old still work via an alias: https://gist.github.com/AndreasBackx/6025e91844e3d766d4264a01ae4d1a71

This seems like a tiny improvement? I plan to make further changes later, but I want to keep PRs separate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants