wayshot icon indicating copy to clipboard operation
wayshot copied to clipboard

CLI: simplify and make friendlier and more Unix-like

Open murlakatamenka opened this issue 11 months ago • 8 comments

Here are some of the issues:

  1. Long flags should be made kebab case for readability:
  • --listoutputs -> --list-outputs
  • --choseoutputs -> --choose-outputs

cp, mv, curl, wget, rg have it like that.

  1. --file <PATH> and --stdout can be replaced with optional [FILE] argument.
match `FILE` {
    Some('-') => // stdout
    Some(path) => // write to path
    None => {
       // write <unixtime>-wayshot.png to current dir
       // print "Saved to <path>" to stdin/stdout, to make the default invocation obvious
    }
}

- for stdout is a common Unix convention.

  1. Help (--help)
  • move --debug to the end of the help, because that's not what a user typically looks for when learning how to use a new tool
  • specify supported extensions. Now they are only mentioned in README and manpage
  • colors! Clap v4 can have clap v3 colored help or custom style, for example, I like rustic's (permalink)
  1. --list-outputs

Currently, output of --list-outputs isn't suitable to pipe to another selector (fzf/skim or GUI dmenu-like fuzzel, wofi), because it's done via tracing:

https://github.com/waycrate/wayshot/blob/cb6bd68dbbe6ab70a5d8fe3bd04cc154f0631cd8/wayshot/src/wayshot.rs#L75-L81

It's output to stderr and exits with an error code 1 for no reason.

Should be: just 1 output per line to stdout, and 0 exit code. Good for:

wayshot --output "$(wayshot --list-outputs | fuzzel --dmenu)"

to be bound to a specific hotkey, like PrintScreen.

  1. Add TAB-completion

clap is used for CLI, clap_complete is one step away. Completion scripts can be generated during build time, there is already bulid.rs in the project


I'll make a draft PR to address those issues, to me all of them are straight improvements of wayshot's UX.

murlakatamenka avatar Mar 12 '24 17:03 murlakatamenka

There's already a CLI UX rewrite in the freeze-feat branch slated for merge. Some of these concerns have been addressed there so it may be better to build off that.

CheerfulPianissimo avatar Mar 12 '24 17:03 CheerfulPianissimo

@CheerfulPianissimo true that, I'll take a look and give feedback (and contributions, if needed) there. Thanks.


I see it's been rewritten with clap+derive, and "-" is used for stdout, pretty good :+1:

murlakatamenka avatar Mar 12 '24 21:03 murlakatamenka

These seem to be the only issues left in the freeze branch:

  1. --list-outputs: I think I missed actually fixing the implementation for it, but it shouldn't be hard to add.
  2. Tab completion: like you said clap_complete should do. I'm unsure if cargo would be able to install it but package managers are able to. We'll need to include them in the releases which shouldn't be hard. Let me know if anyone wants to implement this, I can give some help on the things you need to look out for. The most important part is that it's generated based on an environment variable, not based on the CLI arguments. I currently don't have the time for this myself right now however.

AndreasBackx avatar Mar 15 '24 00:03 AndreasBackx

I've just noticed, that #104 partially breaks what's being asked in this issue, the - instead of stdout part, to be precise. While implementing config file, I though that it would be great to give users more freedom with screenshot output options, so users can have a file output and stdout output at the same time, if they want to (when using - in place of file user can't do that). So I brought back the stdout parameter instead of - and slightly changed the overall flow

Gigas002 avatar Apr 02 '24 12:04 Gigas002

@Gigas002

While implementing config file, I though that it would be great to give users more freedom with screenshot output options, so users can have a file output and stdout output at the same time, if they want to (when using - in place of file user can't do that). So I brought back the stdout parameter instead of - and slightly changed the overall flow

I disagree that one should send output to the stdout AND to an additional file. This is confusing and not implemented anywhere else. Could you give a good usecase where this behaviour is desired?

AndreasBackx avatar Apr 07 '24 00:04 AndreasBackx

Also the tee tool exists precisely for this functionality. Piping stdout to it seems to me the more UNIX-ey solution.

CheerfulPianissimo avatar Apr 07 '24 05:04 CheerfulPianissimo

I find myself asking the question: "why not?" It leads to no extra overhead and is literally changing one else if to a separate if block

Shinyzenith avatar Apr 07 '24 06:04 Shinyzenith

But there's also the question of "is anyone going to realistically use this?"

Shinyzenith avatar Apr 07 '24 06:04 Shinyzenith