Fix non-working "-n" or "--nonotify" switch
Needs ReviewPublic

Authored by sharvey on Feb 25 2018, 2:56 AM.

Details

Reviewers
ngraham
rkflx
Group Reviewers
Spectacle
Summary

The -n/--nonotify switch was not working properly; the GUI would still be shown. Fix is to set Spectacle to "BackgroundMode" if "nonotify" is set.

Diff Detail

Repository
R166 Spectacle
Branch
nonotify (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
sharvey requested review of this revision.Feb 25 2018, 2:56 AM
sharvey created this revision.

Thanks for the patch! Reading the --help text, it seems like this is an option designed to be run in conjunction with --background` mode:

-n, --nonotify In background mode, do not pop up a notification

And indeed, spectacle --background --nonotify does what you would expect.

This could certainly be made clearer in the --help text, perhaps by adding an "If" to the beginning.

I think there's a similar problem with -o/--output, which reads:

-o, --output <fileName> In background mode, save image to specified file

Instead, it opens the GUI with the screenshot as "Untitled".

My reading of that option would lead me to believe it will grab the screen and automatically save it without any further input from the user. And probably with no notification. Just a quick snap-n-save.

Thoughts?

For these options that only work when --background is also used, we should do one of the following:

  • Also invoke --background automatically, and update the help text to indicate that this will happen
  • Make it more clear that the option only works when --background is used.

I'd slightly prefer the first.

From a user's point of view, I didn't even know Spectacle had CLI options until I started working on the code yesterday.

I think the -b/--background flag is confusing and unnecessary. I believe the parser flags and help text could be reworked to eliminate the need for the -b switch entirely. A single flag for each of the different CLI modes; no need to specify "background mode".

It's just a matter of swapping around a few if/else clauses in the launch code to make this work.

I'm willing to put it together if there's agreement.

From a user's point of view, I didn't even know Spectacle had CLI options until I started working on the code yesterday.

I think the -b/--background flag is confusing and unnecessary. I believe the parser flags and help text could be reworked to eliminate the need for the -b switch entirely. A single flag for each of the different CLI modes; no need to specify "background mode".

Sounds sane to me!

I'm working on revamping the command-line options and hope to have something to upload this evening or tomorrow morning. I've rewritten several of the help strings to be clearer about how to use them. Stay tuned.

rkflx resigned from this revision.Aug 25 2018, 6:42 AM