search

resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
495 stars 49 forks source link

Add `--list-options` and shell completions #377

Closed N-R-K closed 4 months ago

N-R-K commented 4 months ago

This adds a --list-options flag which outputs a specialized TSV of all the options intended to be used in shell completion scripts. A couple TODOs:

N-R-K commented 4 months ago

make install should install the zsh completion script

Not sure how to tackle this one. Shell completion script dirs are not as well "standardized" as /usr/local/. For example, older zsh version doesn't recognize /usr/local/share/zsh/site-functions by default. On the other hand, installing into /usr/share.. violates the assumption that make install files goes to /usr/local....

Maybe we shouldn't try to install the completion files and just let distro packagers install it to appropriate directory for the respective distro. Or make it so that completion files will be installed only when explicitly requested via something like ./configure --with-zsh-completion-path="..." or make install ZSH_COMP_PATH="...".

N-R-K commented 4 months ago

Should --help output also include (human readable) list of options??

I'm thinking we could probably just remove the current usage string entirely and utilize the new --list-options for help string:

$ ./scrot -h
Usage:  scrot [OPTIONS] [FILE]

OPTIONS
=======
-a, --autoselect <x,y,w,h>      autoselect provided region
-b, --border                    capture the window borders as well
-C, --class <NAME>              capture specified window class
-c, --count                     display a countdown for delay
-D, --display <DISPLAY>         capture specified display
-d, --delay <[b]SEC>            add delay before screenshot
-e, --exec <CMD>                execute command on saved image
-F, --file <FILE>               specify output file
-f, --freeze                    freeze the screen when -s is used
-h, --help                      display help and exit
-i, --ignorekeyboard            ignore keyboard
-k, --stack[=v|h]               capture overlapped window and join them
-l, --line <STYLE>              specify the style of the selection line
-M, --monitor <NUM>             capture monitor
-m, --multidisp                 capture all monitors
-o, --overwrite                 overwrite the output file if needed
-p, --pointer                   capture the mouse pointer as well
-q, --quality <NUM>             image quality
-s, --select[=OPTS]             interactively select a region to capture
-t, --thumb <% | WxH>           also generate a thumbnail
-u, --focused                   capture the currently focused window
-u, --focussed                  capture the currently focused window
-v, --version                   output version and exit
-w, --window <WID>              X window ID to capture
-Z, --compression <LVL>         image compression level
-z, --silent                    prevent beeping
--format <FMT>                  specify output file format
--list-options[=human|tsv]      list all options

This seems significantly more helpful than the current -h output. It also includes long opt only options like --format which the older help string didn't.

EDIT: Updated the --help output to the above. Feedback welcome.

N-R-K commented 4 months ago

Any feedback on this? I'd like to merge this and cut a release sometime this week.

daltomi commented 4 months ago

This seems significantly more helpful than the current -h output

Yes I agree. Previously the format was like this but then it was decided to change it, see: b4ca02e ("use the manual's synopsis as help text (#104)", 2021-07-28) I don't currently have a preference.

daltomi commented 4 months ago

In this one, as in previous commits ;-) the copyright date needs to be updated.

daltomi commented 4 months ago

make install ZSH_COMP_PATH="...".

I like this one better, if possible, I don't know. But making a target in Makefile ala make install-shell or something like that would be best.

daltomi commented 4 months ago

It would be necessary to add the etc/ directory, so that when doing make dist it would include it.

diff --git a/Makefile.am b/Makefile.am
index 63e275e..497b212 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -40,6 +40,8 @@ dist_doc_DATA = AUTHORS ChangeLog CONTRIBUTING.md doc/scrot.png FAQ.md README.md

 EXTRA_DIST = $(man_MANS) autogen.sh deps.pc

+EXTRA_DIST += etc/
+
 SUBDIRS = src

 distclean-local:
N-R-K commented 4 months ago

It would be necessary to add the etc/ directory, so that when doing make dist it would include it.

Good catch. Fixed.

the copyright date needs to be updated.

I will go through all the commits since the previous version and update the copyright date in another commit.

"use the manual's synopsis as help text (https://github.com/resurrecting-open-source-projects/scrot/pull/104)"

That was okay in the past, but currently we have long options which have no short options (e.g --format). Showing only short opt excludes these long only opt.

daltomi commented 4 months ago

Or make it so that completion files will be installed only when explicitly requested via something like ./configure --with-zsh-completion-path="..."

Hi, I was doing some tests and as you indicate it can be done (It is similar but with environment variable), but not with make install-shell as I would like :-/ I attach the patch. scrot.txt This works as follows. First, clarify that the names of the variables/options are vague, it's just an idea.

The new options are:

 --with-zsh-confpath     install autocomplete zsh files
 --with-bash-confpath    install autocomplete bash files

Example:

$ ./configure --with-zsh-confpath
configure: error: the variable ZSH_CONFPATH no set

It expects to find a ZSH_CONFPATHenvironment variable defined. We define it like this( or whatever, export, set, etc)

$  ./configure --prefix=/usr/local --with-zsh-confpath ZSH_CONFPATH=/tmp/zsh-autocompletion

Then we install to test:

make DESTDIR=/tmp/scrot install

It is installed on: /tmp/scrot/usr/local and /tmp/zsh-autocompletion/ Note that the --prefix option had no impact on the installation of zsh

That is an approximation, it would be necessary to see if there is a better or more robust one because this patch does not uninstall the shell files for example.

N-R-K commented 4 months ago

Note that the --prefix option had no impact on the installation of zsh

I think that's probably fine, but I'm not sure.

configure: error: the variable ZSH_CONFPATH no set

Can we make it so that the path is given as an argument? E.g:

$ ./configure --with-zsh-confpath=/tmp/zsh-complete

I think this is more easy to use.

N-R-K commented 4 months ago

I've merged this PR since the autocompletion itself works.

I attach the patch.

For installing the files, you can now create your own PR. I'm not good with autoconf, so I wouldn't be able to do it myself anyways.

daltomi commented 4 months ago

El Tue, 04 Jun 2024 00:06:27 -0700 NRK @.***> escribió:

I've merged this PR since the autocompletion itself works.

I attach the patch.

For installing the files, you can now create your own PR. I'm not good with autoconf, so I wouldn't be able to do it anyways.

Ok, as soon as I can I'll make a PR and we'll continue it there.

Regards.

N-R-K commented 4 months ago

@daltomi I was thinking it might be time for a release. Do you think we should wait to add autoconf/makefile completion installation before making a release?

I think most distros/package-managers will not have any trouble installing those files. It will be useful only for people who are not using package manager and using make install directly.

daltomi commented 4 months ago

El Sun, 09 Jun 2024 11:28:23 -0700 NRK @.***> escribió:

@daltomi I was thinking it might be time for a release. Do you think we should wait to add autoconf/makefile completion installation before making a release?

I think most distros/package-managers will not have any trouble installing those files.

Hi, Yes, it's okay with me.