3
\$\begingroup\$

I haven't found any Lightshot Print Screen key linux handler publicly available.

So, I decided to write one today. It is a standard POSIX shell script.

I believe there is always space to get better, so any and all reviews are welcome.


#!/bin/sh

is_number()
{
    # check if exactly one argument has been passed
    test "$#" -eq 1 || print_error_and_exit 5 "is_number(): There has not been passed exactly one argument!"

    # check if the argument is an integer
    test "$1" -eq "$1" 2>/dev/null
}

# ------------------------------------------------------------------------------

print_error_and_exit()
{
    # check if exactly two arguments have been passed
    test "$#" -eq 2 || print_error_and_exit 3 "print_error_and_exit(): There have not been passed exactly two arguments!"

    # check if the first argument is a number
    is_number "$1" || print_error_and_exit 4 "print_error_and_exit(): The argument #1 is not a number!"

    # check if we have color support
    if test -x /usr/bin/tput && tput setaf 1 > /dev/null 2>&1
    then
        bold=$(tput bold)
        red=$(tput setaf 1)
        nocolor=$(tput sgr0)
        echo "$bold$red$2 Exit code = $1.$nocolor" 1>&2
    else
        echo "$2 Exit code = $1." 1>&2
    fi

    exit "$1"
}

# ------------------------------------------------------------------------------

check_for_prerequisite()
{
    # check if exactly one argument has been passed
    test "$#" -eq 1 || print_error_and_exit 2 "check_for_prerequisite(): There has not been passed exactly one argument!"

    # check if the argument is a program which is installed
    command -v "$1" > /dev/null 2>&1 || print_error_and_exit 6 "check_for_prerequisite(): I require $1 but it's not installed :-("
}

# ------------------------------------------------------------------------------

# check if no arguments have been passed to the script
test "$#" -gt 0 && print_error_and_exit 1 "$0: You have passed some unexpected argument(s) to the script!"

# ------------------------------------------------------------------------------

# check for prerequisites
check_for_prerequisite "pgrep"
check_for_prerequisite "xdotool"

# ------------------------------------------------------------------------------

# global constants
lightshot_key="Print"
lightshot_name="Lightshot"

# ------------------------------------------------------------------------------

# get the lightshot pid
lightshot_pid=$(pgrep "$lightshot_name")

# test if a pid has been successfully acquired
is_number "$lightshot_pid" || print_error_and_exit 7 "lightshot_pid: The argument is not a number!\\nLightshot is most probably not running."

# ------------------------------------------------------------------------------

# get the window id from lightshot pid
lightshot_wnd=$(xdotool search --limit 1 --all --pid "$lightshot_pid" --name "$lightshot_name")

# test if a window handler has been successfully acquired
is_number "$lightshot_wnd" || print_error_and_exit 8 "lightshot_wnd: The argument is not a number!\\nLightshot is most probably not running."

# ------------------------------------------------------------------------------

# simulate a print screen key press on the lightshot window
xdotool key --window "$lightshot_wnd" "$lightshot_key"

In Linux Mint 18.3 you can simply put this into your custom keyboard shortcuts:

Screenshot from Linux Mint 18.3

\$\endgroup\$
1

1 Answer 1

2
\$\begingroup\$

Instruct shell how to treat unset variables

set -o nounset

In this script, there should not be possible to encounter an unbound variable, so setting this will prove useful for debugging.


Redirection style

 9     test "$1" -eq "$1" 2>/dev/null

in contrast with:

23     if test -x /usr/bin/tput && tput setaf 1 > /dev/null 2>&1

It should be unified either with, or without a space.


Constants should be moved to the top

61 lightshot_key="Print"
62 lightshot_name="Lightshot"

It is obvious, that everyone might not want to use Print, so this really has to be moved to the top in order for each user to quickly customize the keyboard combination for the print screen.


Those constants should be commented and clarified

In order for the users to quickly know, how to define key combinations, there should be some example added.


Consider replacing echo with printf

28         echo "$bold$red$2 Exit code = $1.$nocolor" 1>&2
29     else
30         echo "$2 Exit code = $1." 1>&2

Basically, it's a portability (and reliability) issue. Read more about this topic in this answer.


Problematic code

I notice now, there is a new line inside these strings:

70 is_number "$lightshot_pid" || print_error_and_exit 7 "lightshot_pid: The argument is not a number!\\nLightshot is most probably not running."
78 is_number "$lightshot_wnd" || print_error_and_exit 8 "lightshot_wnd: The argument is not a number!\\nLightshot is most probably not running."

Which according to Shellcheck.net:

echo won't expand escape sequences

Read more about this topic in this article.

This code, as it is, works though, in spite of this error. Tested in bash version 4.3.48(1)-release and POSIX dash version 0.5.8-2.1ubuntu2.


Better variable naming convention

These are of little value to a foreign script reader:

61 lightshot_key="Print"
62 lightshot_name="Lightshot"

There are only two constants. Why not to give them some real meaning, this would further enhance the script reader's experience.

The variables:

67 lightshot_pid
75 lightshot_wnd

could use some love too.


Tell the user what arguments are (not) expected

50 test "$#" -gt 0 && print_error_and_exit 1 "$0: You have passed some unexpected argument(s) to the script!"

Let's add there something like:

No arguments expected!


Problematic space in the comments (block separators)

12 # ------------------------------------------------------------------------------

There should be no space between # and -, because imagine, that you have a larger project than this, having these block separators will prevent you from searching for long arguments beginning (--).


Re-consider using smileys in the output and using grammar shortcuts

44     command -v "$1" > /dev/null 2>&1 || print_error_and_exit 6 "check_for_prerequisite(): I require $1 but it's not installed :-("

It may look a little unprofessional. Hard to say without knowing the people reading your script, though. (It might be totally OK for personal usage.) This script will likely be spread across the internet, so better not use that. Also, avoid using grammar shortcuts like:

it's


xdotool: documenting all passed arguments would be greatly appreciated by the script readers (and the order of arguments in lightshot_wnd could be better a little)

75 lightshot_wnd=$(xdotool search --limit 1 --all --pid "$lightshot_pid" --name "$lightshot_name")

Re-writen code would look as follows

#!/bin/sh

# treat unset variables as an error when substituting
set -o nounset

# global constants for an easy set-up:
    # lightshot_printscreen_hotkey: set this to the same hotkey which you have set up in Lightshot
    #                               example: for left control and print screen key -> type Control_L+Print
lightshot_printscreen_hotkey="Print"
    # lightshot_process_name: do not change this one, it is a case-sensitive name of the Lightshot process
lightshot_process_name="Lightshot"

#------------------------------------------------------------------------------

is_number()
{
    # check if exactly one argument has been passed
    test "$#" -eq 1 || print_error_and_exit 5 "is_number(): There has not been passed exactly one argument!"

    # check if the argument is an integer
    test "$1" -eq "$1" 2> /dev/null
}

#------------------------------------------------------------------------------

print_error_and_exit()
{
    # check if exactly two arguments have been passed
    test "$#" -eq 2 || print_error_and_exit 3 "print_error_and_exit(): There have not been passed exactly two arguments!"

    # check if the first argument is a number
    is_number "$1" || print_error_and_exit 4 "print_error_and_exit(): The argument #1 is not a number!"

    # check if we have color support
    if test -x /usr/bin/tput && tput setaf 1 > /dev/null 2>&1
    then
        bold=$(tput bold)
        red=$(tput setaf 1)
        nocolor=$(tput sgr0)
        printf "%s%s%s Exit code = %s.%s\\n" "$bold" "$red" "$2" "$1" "$nocolor" 1>&2
    else
        printf "%s Exit code = %s.\\n" "$2" "$1" 1>&2
    fi

    exit "$1"
}

#------------------------------------------------------------------------------

check_for_prerequisite()
{
    # check if exactly one argument has been passed
    test "$#" -eq 1 || print_error_and_exit 2 "check_for_prerequisite(): There has not been passed exactly one argument!"

    # check if the argument is a program which is installed
    command -v "$1" > /dev/null 2>&1 || print_error_and_exit 6 "check_for_prerequisite(): I require $1 but it is not installed!"
}

#------------------------------------------------------------------------------

# check if no arguments have been passed to the script
test "$#" -gt 0 && print_error_and_exit 1 "$0: You have passed some unexpected argument(s) to the script. No arguments expected!"

#------------------------------------------------------------------------------

# check for prerequisites
check_for_prerequisite "pgrep"
check_for_prerequisite "xdotool"

#------------------------------------------------------------------------------

# get the Lightshot process id
lightshot_process_id=$(pgrep "$lightshot_process_name")

# test if a process id has been successfully acquired
is_number "$lightshot_process_id" || print_error_and_exit 7 "lightshot_process_id: The argument is not a number! Lightshot is most probably not running."

#------------------------------------------------------------------------------

# get the window id from the Lightshot process id
    #--all   : Require that all conditions be met.
    #--limit : Stop searching after finding N matching windows.
    #--pid   : Match windows that belong to a specific process id.
    #--name  : Match against the window name. This is the same string that is displayed in the window titlebar.
lightshot_window_id=$(xdotool search --all --limit 1 --pid "$lightshot_process_id" --name "$lightshot_process_name")

# test if a window id has been successfully acquired
is_number "$lightshot_window_id" || print_error_and_exit 8 "lightshot_window_id: The argument is not a number! Lightshot is most probably not running."

#------------------------------------------------------------------------------

# simulate the above pre-defined print screen hotkey press on the Lightshot window
xdotool key --window "$lightshot_window_id" "$lightshot_printscreen_hotkey"
\$\endgroup\$
1
  • \$\begingroup\$ Nice self-review. I think your variable names are fine. As for echo vs printf, it's best to use echo without any flags when possible. The flags of echo are not portable, but echo itself is fine. It's not simple to recommend printf for everything because it's not POSIX. So if you want to embed \n in a string, which would require either printf or echo -e, then it's best to rewrite with two echo. \$\endgroup\$
    – janos
    Jun 30, 2018 at 6:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.