[pacman-dev] [PATCH] Improve --help switch output for pacman contrib and pacman scripts

Andrew Gregory andrew.gregory.8 at gmail.com
Thu Oct 31 23:52:08 EDT 2013


On 10/31/13 at 03:40pm, Jason St. John wrote:
> Unify the formatting of the --help switch for pacman utils, if it exists.
> All of the pacman utils will now output help text using the following
> format:
> 
>   util-name (pacman) v<pacman version>
> 
>   one line description of util's purpose
> 
>   Usage: util-name [options]
> 
>     -b, --bar      whatever --bar does
>     -f, --foo      whatever --foo does
>     -h, --help     display this help message
> 
> Reported-by: Karol Błażewicz <karol.blazewicz at gmail.com>
> Signed-off-by: Jason St. John <jstjohn at purdue.edu>
> ---
> This patch is analogous to commit a86015f except this one deals with pacman
> contrib and pacman scripts instead of pacman utils.
> 
>  contrib/bacman.sh.in            |  7 ++++---
>  contrib/checkupdates.sh.in      | 10 +++++++---
>  contrib/paccache.sh.in          | 16 +++++++++-------
>  contrib/pacdiff.sh.in           |  8 +++++---
>  contrib/paclist.sh.in           |  7 ++++---
>  contrib/paclog-pkglist.sh.in    |  8 +++++---
>  contrib/pacscripts.sh.in        |  9 +++++----
>  contrib/pacsearch.in            |  5 +++--
>  contrib/pacsysclean.sh.in       |  6 ++++--
>  contrib/rankmirrors.sh.in       | 13 +++++++++----
>  contrib/updpkgsums.sh.in        | 13 +++++++------
>  scripts/makepkg.sh.in           |  2 ++
>  scripts/pacman-db-upgrade.sh.in |  1 +
>  scripts/pkgdelta.sh.in          |  4 ++--
>  14 files changed, 67 insertions(+), 42 deletions(-)

You have a lot of echo's with "\n" in them.  Bash's echo doesn't
handle escapes without -e and we're fairly consistent about using
an empty echo for blank lines, which has the added benefit that the
code more closely resembles what the output will look like..

I've also included a suggestions to improve wording in a few places.

> 
> diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
> index 74a83bc..94487b8 100644
> --- a/contrib/bacman.sh.in
> +++ b/contrib/bacman.sh.in
> @@ -37,9 +37,10 @@ m4_include(../scripts/library/output_format.sh)
>  # User Friendliness
>  #
>  usage() {
> -	echo "This program recreates a package using pacman's db and system files"
> -	echo "Usage:   $myname [--nocolor] [--pacnew] <installed package name>"
> -	echo "Example: $myname kernel26"
> +	echo "${myname} (pacman) v${myver}\n\n"
> +	echo "Recreate a package using pacman's database and system files\n\n"
> +	echo "Usage: ${myname} [--nocolor] [--pacnew] <installed package name>\n\n"
> +	echo "Example: ${myname} linux-headers"
>  }

echo "\n"

>  
>  version() {
> diff --git a/contrib/checkupdates.sh.in b/contrib/checkupdates.sh.in
> index 48ceff9..ce6bc30 100644
> --- a/contrib/checkupdates.sh.in
> +++ b/contrib/checkupdates.sh.in
> @@ -18,10 +18,14 @@
>  #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  #
>  
> +declare -r myname='checkupdates'
> +declare -r myver='@PACKAGE_VERSION@'
> +
>  if (( $# > 0 )); then
> -	echo "checkupdates: Safely print a list of pending updates."
> -	echo "Use: checkupdates"
> -	echo "Export the 'CHECKUPDATES_DB' variable to change the path of the temporary database."
> +	echo "${myname} (pacman) v${myver}\n\n"
> +	echo "Safely print a list of pending updates\n\n"
> +	echo "Usage: ${myname}\n\n"
> +	echo 'Note: Export the "CHECKUPDATES_DB" variable to change the path of the temporary database.'
>  	exit 0
>  fi

echo "\n"

>  
> diff --git a/contrib/paccache.sh.in b/contrib/paccache.sh.in
> index 2bbe726..bcabaa4 100644
> --- a/contrib/paccache.sh.in
> +++ b/contrib/paccache.sh.in
> @@ -152,27 +152,29 @@ summarize() {
>  
>  usage() {
>  	cat <<EOF
> -usage: $myname <operation> [options] [targets...]
> +${myname} (pacman) v${myver}
>  
>  $myname is a flexible pacman cache cleaning utility, which has numerous
>  options to help control how much, and what, is deleted from any directory
>  containing pacman package tarballs.

This does not match your template of a one line description.  How
about just "A flexible pacman cache cleaning utility"? The user can
see the "numerous options" immediately below.

>  
> +Usage: ${myname} <operation> [options] [targets...]
> +
>    Operations:
>      -d, --dryrun          perform a dry run, only finding candidate packages.
> -    -m, --move <dir>      move candidate packages to 'movedir'.
> +    -m, --move <dir>      move candidate packages to "movedir".
>      -r, --remove          remove candidate packages.
>  
>    Options:
> -    -a, --arch <arch>     scan for 'arch' (default: all architectures).
> -    -c, --cachedir <dir>  scan 'cachedir' for packages.
> +    -a, --arch <arch>     scan for "arch" (default: all architectures).
> +    -c, --cachedir <dir>  scan "cachedir" for packages.
>                            (default: @localstatedir@/cache/pacman/pkg).
>      -f, --force           apply force to mv(1) and rm(1) operations.
>      -h, --help            display this help message and exit.
> -    -i, --ignore <pkgs>   ignore 'pkgs', comma separated. Alternatively, specify
> -                          '-' to read package names from stdin, newline
> +    -i, --ignore <pkgs>   ignore "pkgs", comma-separated. Alternatively, specify
> +                          "-" to read package names from stdin, newline-
>                            delimited.
> -    -k, --keep <num>      keep 'num' of each package in 'cachedir' (default: 3).
> +    -k, --keep <num>      keep "num" of each package in "cachedir" (default: 3).
>      --nocolor             remove color from output.
>      -u, --uninstalled     target uninstalled packages.
>      -v, --verbose         increase verbosity. specify up to 3 times.
> diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in
> index 6e70fee..4628e83 100644
> --- a/contrib/pacdiff.sh.in
> +++ b/contrib/pacdiff.sh.in
> @@ -33,20 +33,22 @@ m4_include(../scripts/library/output_format.sh)
>  
>  usage() {
>  	cat <<EOF
> -$myname is a simple pacnew/pacorig/pacsave updater.
> +${myname} (pacman) v${myver}
> +
> +A simple pacnew/pacorig/pacsave updater.

Is this an accurate description?  It doesn't "update" the
pac{new,orig,save} files, it merges and even *removes* them.

>  
>  Usage: $myname [-l | -f | -p] [--nocolor]
>  
>  Search Options:     select one, default: pacmandb
>    -l/--locate       scan using locate
>    -f/--find         scan using find
> -  -p/--pacmandb     scan active config files from pacman db
> +  -p/--pacmandb     scan active config files from pacman database
>  
>  General Options:
>    -o/--output       print files instead of merging them
>    --nocolor         remove colors from output
>  
> -Enviroment Variables:
> +Environment Variables:
>    DIFFPROG          override the merge program: (default: vimdiff)
>    DIFFSEARCHPATH    override the search path. (only when using find)
>                      (default: /etc)
> diff --git a/contrib/paclist.sh.in b/contrib/paclist.sh.in
> index 7883e21..dc90b06 100644
> --- a/contrib/paclist.sh.in
> +++ b/contrib/paclist.sh.in
> @@ -31,9 +31,10 @@ if ! type gettext &>/dev/null; then
>  fi
>  
>  usage() {
> -	printf '%s - List all packages installed from a given repo\n' "$myname"
> -	printf 'Usage:   %s <repo>\n' "$myname"
> -	printf 'Example: %s testing\n' "$myname"
> +	printf "%s (pacman) v%s\n\n" "${myname}" "${myver}"
> +	printf "List all packages installed from a given repository\n\n" "${myname}"
> +	printf "Usage: %s <repository>\n\n" "${myname}"
> +	printf "Example: %s testing\n" "${myname}"
>  }
>  
>  version() {
> diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in
> index 9b3abad..6adfbbe 100644
> --- a/contrib/paclog-pkglist.sh.in
> +++ b/contrib/paclog-pkglist.sh.in
> @@ -25,9 +25,11 @@ export TEXTDOMAINDIR='/usr/share/locale'
>  declare logfile=${1:- at localstatedir@/log/pacman.log}
>  
>  usage() {
> -	printf 'usage:   %s [pacman log]\n' "$myname"
> -	printf 'example: %s @localstatedir@/log/pacman.log\n' "$myname"
> -	printf '\ndefaults to: @localstatedir@/log/pacman.log\n'
> +	printf "%s (pacman) v%s\n\n" "${myname}" "${myver}"
> +	printf "Parse a log file into a list of currently installed packages\n\n"
> +	printf "Usage: %s [path to pacman log]\n\n" "${myname}"
> +	printf "Example: %s @localstatedir@/log/pacman.log\n\n" "${myname}"
> +	printf 'Defaults to: @localstatedir@/log/pacman.log'
>  }
>  
>  version() {
> diff --git a/contrib/pacscripts.sh.in b/contrib/pacscripts.sh.in
> index 62c4e35..17fbe29 100644
> --- a/contrib/pacscripts.sh.in
> +++ b/contrib/pacscripts.sh.in
> @@ -46,11 +46,12 @@ error() {
>  }
>  
>  usage() {
> +	echo "${myname} (pacman) v${myver}\n"
>  	echo "This program prints out the {pre,post}_{install,remove,upgrade} scripts"
> -	echo "of a given package."

echo "\n"

"This program" is unnecessary.  How about shortening it to "Print the
{pre..." and putting it all on one line?

> -	echo "Usage: $myname pkgname|pkgfile"
> +	echo "of a given package.\n"
> +	echo "Usage: ${myname} <pkgname|pkgfile>"
>  	echo
> -	echo " OPTIONS:"
> +	echo " Options:"
>  	echo "  -h, --help                 Print this help message"
>  	echo "  -v, --version              Print program name and version"
>  	echo
> @@ -70,7 +71,7 @@ spacman() {
>  	else
>  		if ! type -p sudo; then
>  			error "Cannot find the sudo binary! Is sudo installed?"
> -			error "Otherwise try to run the program as root"
> +			error "Otherwise, try to run the program as root"
>  			exit 1
>  		else
>  			sudo pacman "$@"
> diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in
> index b1db8ab..624f201 100644
> --- a/contrib/pacsearch.in
> +++ b/contrib/pacsearch.in
> @@ -28,8 +28,9 @@ my $myname = 'pacsearch';
>  my $myver = '@PACKAGE_VERSION@';
>  
>  sub usage {
> -	print "$myname - Add color and install information to a pacman -Ss search\n";
> -	print "Usage:   $myname <pattern>\n";
> +	print "${myname} (pacman) v${myver}\n\n";
> +	print "Add color and install information to a 'pacman -Ss' search\n\n";
> +	print "Usage: ${myname} <pattern>\n\n";
>  	print "Example: $myname ^gnome\n";
>  }
>  
> diff --git a/contrib/pacsysclean.sh.in b/contrib/pacsysclean.sh.in
> index 37219ab..9ec428c 100644
> --- a/contrib/pacsysclean.sh.in
> +++ b/contrib/pacsysclean.sh.in
> @@ -8,7 +8,10 @@ declare -r myver='@PACKAGE_VERSION@'
>  PACMAN_OPTS=
>  
>  usage() {
> -	echo "$myname - Sort installed packages by increasing installed size."
> +	echo "${myname} (pacman) v${myver}"
> +	echo
> +	echo "Sort installed packages by increasing installed size. Useful for"
> +	echo "system clean-up."
>  	echo
>  	echo "Usage: $myname [options]"
>  	echo
> @@ -22,7 +25,6 @@ version() {
>  	echo 'Copyright (C) 2011 Eric Bélanger <snowmaniscool at gmail.com>'
>  }
>  
> -
>  if [ -n "$1" ]; then
>  	case "$1" in
>  		-o) PACMAN_OPTS="${2}" ;;
> diff --git a/contrib/rankmirrors.sh.in b/contrib/rankmirrors.sh.in
> index 5555edf..f54369f 100644
> --- a/contrib/rankmirrors.sh.in
> +++ b/contrib/rankmirrors.sh.in
> @@ -21,26 +21,31 @@
>  # traps interrupt key to spit out pre-interrupt info
>  trap finaloutput INT
>  
> +declare -r myname='rankmirrors'
> +declare -r myver='@PACKAGE_VERSION@'
> +
>  usage() {
> -	echo "Usage: rankmirrors [options] MIRRORFILE | URL"
> +	echo "${myname} (pacman) v${myver}"
>  	echo
>  	echo "Ranks pacman mirrors by their connection and opening speed. Pacman mirror"
>  	echo "files are located in @sysconfdir@/pacman.d/. It can also rank one mirror if the URL is"
>  	echo "provided."
>  	echo
> +	echo "Usage: ${myname} [options] MIRRORFILE | URL"
> +	echo
>  	echo "Options:"
>  	echo "  --version      show program's version number and exit"
>  	echo "  -h, --help     show this help message and exit"
>  	echo "  -n NUM         number of servers to output, 0 for all"
>  	echo "  -t, --times    only output mirrors and their response times"
> -	echo "  -u, --url      test a specific url"
> +	echo "  -u, --url      test a specific URL"
>  	echo "  -v, --verbose  be verbose in ouptut"
> -	echo "  -r, --repo     specify a specific repo name instead of guessing"
> +	echo "  -r, --repo     specify a specific repository name instead of guessing"

"specify a specific" seems a little redundant.

>  	exit 0
>  }
>  
>  version() {
> -	echo "rankmirrors (pacman) @PACKAGE_VERSION@"
> +	echo "${myname} (pacman) ${myver}"
>  	echo "Copyright (c) 2009 Matthew Bruenig <matthewbruenig at gmail.com>."
>  	echo
>  	echo "This is free software; see the source for copying conditions."
> diff --git a/contrib/updpkgsums.sh.in b/contrib/updpkgsums.sh.in
> index 4d35357..130cd3f 100644
> --- a/contrib/updpkgsums.sh.in
> +++ b/contrib/updpkgsums.sh.in
> @@ -23,12 +23,13 @@ declare -r myname='updpkgsums'
>  declare -r myver='@PACKAGE_VERSION@'
>  
>  usage() {
> -	printf 'usage: %s [buildfile]\n\n' "$myname"
> -	printf '    -h, --help        display this help message and exit\n'
> -	printf '    -V, --version     display version information and exit\n\n'
> -	printf '%s will perform an in place update the checksums in the\n' "$myname"
> -	printf 'path specified by [buildfile], defaulting to PKGBUILD in the current\n'
> -	printf 'working directory.\n'
> +	printf "%s (pacman) v%s\n\n" "${myname}" "${myver}"
> +	printf "%s will perform an in place update of the checksums in the\n" "${myname}"
> +	printf "path specified by [build file], defaulting to PKGBUILD in the current\n"
> +	printf "working directory.\n\n"
> +	printf "Usage: %s [build file]\n\n" "${myname}"
> +	printf "    -h, --help        display this help message and exit\n"
> +	printf "    -V, --version     display version information and exit\n\n"
>  }
>  
>  version() {
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 4cb8173..126b5c5 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -2483,6 +2483,8 @@ m4_include(library/parseopts.sh)
>  usage() {
>  	printf "makepkg (pacman) %s\n" "$makepkg_version"
>  	echo
> +	printf -- "$(gettext "Make packages compatible for use with pacman")\n"
> +	echo
>  	printf -- "$(gettext "Usage: %s [options]")\n" "$0"
>  	echo
>  	printf -- "$(gettext "Options:")\n"
> diff --git a/scripts/pacman-db-upgrade.sh.in b/scripts/pacman-db-upgrade.sh.in
> index a1630c5..739e5f3 100644
> --- a/scripts/pacman-db-upgrade.sh.in
> +++ b/scripts/pacman-db-upgrade.sh.in
> @@ -34,6 +34,7 @@ m4_include(library/output_format.sh)
>  
>  usage() {
>  	printf "pacman-db-upgrade (pacman) %s\n\n" "$myver"
> +	printf -- "$(gettext "Upgrade the local pacman database to a newer format")\n\n"
>  	printf -- "$(gettext "Usage: %s [--nocolor] [pacman_db_root]")\n\n" "$0"
>  }
>  
> diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in
> index 39046b8..7e2fd02 100644
> --- a/scripts/pkgdelta.sh.in
> +++ b/scripts/pkgdelta.sh.in
> @@ -47,9 +47,9 @@ m4_include(library/output_format.sh)
>  # print usage instructions
>  usage() {
>  	printf "pkgdelta (pacman) %s\n\n" "$myver"
> -	printf -- "$(gettext "Usage: pkgdelta [options] <package1> <package2>\n")"
> +	printf -- "$(gettext "Usage: pkgdelta [options] <package1> <package2>\n\n")"

Just use an empty echo for a blank line, no need to force this to be
retranslated.

>  	printf -- "$(gettext "\
> -	pkgdelta will create a delta file between two packages.\n\
> +pkgdelta will create a delta file between two packages.\n\
>  This delta file can then be added to a database using repo-add.\n\n")"
>  	printf -- "$(gettext "Example:  pkgdelta pacman-3.0.0.pkg.tar.gz pacman-3.0.1.pkg.tar.gz")\n"
>  	echo
> -- 
> 1.8.4.2


More information about the pacman-dev mailing list