tmp #84
Closed
tmp #84
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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.
This "improves" (and that is subjective) the design of the CLI. I am aiming to get some feedback on what people think of the new design:
```
Screenshot tool for wlroots based compositors implementing the zwlr_screencopy_v1 protocol.
Usage: wayshot [OPTIONS] [OUTPUT]
Arguments:
[OUTPUT]
Where to save the screenshot, "-" for stdout. Defaults to "$UNIX_TIMESTAMP-wayshot.$EXTENSION"
Options:
--log-level <LOG_LEVEL>
Log level to be used for printing to stderr
[default: info]
[possible values: trace, debug, info, warn, error]
-s, --slurp <SLURP_ARGS>
Arguments to call slurp with for selecting a region
-c, --cursor
Enable cursor in screenshots
--encoding <FILE_EXTENSION>
Set image encoder, if output file contains an extension, that will be used instead
[default: png]
[aliases: extension, format, output-format]
Possible values:
- jpg: JPG/JPEG encoder
- png: PNG encoder
- ppm: PPM encoder
- qoi: Qut encoder
-l, --list-outputs
List all valid outputs
-o, --output <OUTPUT>
Choose a particular output/display to screenshot
--choose-output
Present a fuzzy selector for output/display selection
-h, --help
Print help (see a summary with '-h')
-V, --version
Print version
```
The main changes are:
1. `--debug` is now `--log-level` because this makes it easy to select more specifically what log level to use. I considered using `-v`, `-vv`... to increase verbosity but the `clap-verbosity-crate` uses `log` and not `tracing`. We could use it somewhat, but we'd pull in `log` (which seems fine) and we'd need to map from `log`'s Level to `tracing`'s Level enums (they use inverse ordering).
2. `--stdout` and `--file` has been made an optional positional argument. This because it's what other CLIs often do and I wasn't sure what to call the option otherwise because `--output` and `-O`/`-o` is often what others use but we use it here to refer to displays/monitors/Wayland outputs. This avoids that confusion hopefully and I've also clarified this in the documentation.
* Additionally if a path is given, its extension will always be used. So you cannot save `jpg` to `foo.png`. Perhaps this behaviour can be changed, though I don't see a reason to support this weird edge case? When is someone saving `png` to `jpg`?
3. `--extension` is `--encoding` with aliases like `extension`.
Again, let me know what you think.
Let's bubble up errors instead of panicking and also not use `exit(...)` because it does not call destructors which might give issues.
We use different structs/fields to store the same three types of data everywhere: - Position (x,y) - Size (width,height) - Region(position, size) This makes it so they all reuse the same information.
This was referenced Feb 2, 2024
Closed
Closed
|
Sorry, my mistake. Didn't want to submit this but sapling isn't super user-friendly. |
|
All good! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tmp
Stack created with Sapling. Best reviewed with ReviewStack.