From 06c22678cc124b7df4a5a7dbdd7d060e10111bf5 Mon Sep 17 00:00:00 2001 From: Qubut Date: Sat, 7 Sep 2024 08:16:48 +0200 Subject: [PATCH 1/4] refactor(grimshot): modularize and clean up with functional helpers - Introduced functional helper methods: `when`, `if_else`, and `any` for cleaner, more maintainable logic. - Refactored conditional handling in `notifyOk`, `notifyError`, and `check` functions to leverage the new helper methods. - Moved usage message printing to a dedicated `printUsageMsg` function. - Extracted subject-specific logic (e.g., area, window, screen) into standalone functions: `selectArea`, `selectWindow`, etc. - Improved error handling by introducing functions like `handleScreenshotSuccess`, `handleScreenshotFailure`, and `handleSaveCopy`. - Reformatted code and improved readability by using consistent formatting for case statements and conditions. This refactor enhances code maintainability by isolating logic into reusable components, improving clarity and simplifying future changes. --- functional-helpers | 33 +++++++ grimshot | 229 +++++++++++++++++++++++++++------------------ 2 files changed, 173 insertions(+), 89 deletions(-) create mode 100755 functional-helpers diff --git a/functional-helpers b/functional-helpers new file mode 100755 index 0000000..dcd2213 --- /dev/null +++ b/functional-helpers @@ -0,0 +1,33 @@ +#! /bin/sh +when() { + condition=$1 + action=$2 + + if eval "$condition"; then + eval "$action" + fi +} + +whenOtherwise() { + condition=$1 + true_action=$2 + false_action=$3 + + if eval "$condition"; then + eval "$true_action" + else + eval "$false_action" + fi +} + +any() { + for tuple in "$@"; do + condition=$(echo "$tuple" | cut -d: -f1) + action=$(echo "$tuple" | cut -d: -f2-) + if eval "$condition"; then + eval "$action" + return 0 + fi + done + return 1 # No conditions matched +} diff --git a/grimshot b/grimshot index 5aaf479..7621680 100755 --- a/grimshot +++ b/grimshot @@ -11,9 +11,10 @@ ## Those are needed to be installed, if unsure, run `grimshot check` ## ## See `man 1 grimshot` or `grimshot usage` for further details. +. ./functional-helpers getTargetDirectory() { - test -f "${XDG_CONFIG_HOME:-$HOME/.config}/user-dirs.dirs" && \ + test -f "${XDG_CONFIG_HOME:-$HOME/.config}/user-dirs.dirs" && . "${XDG_CONFIG_HOME:-$HOME/.config}/user-dirs.dirs" echo "${XDG_SCREENSHOTS_DIR:-${XDG_PICTURES_DIR:-$HOME}}" @@ -27,26 +28,26 @@ while [ $# -gt 0 ]; do key="$1" case $key in - -n|--notify) - NOTIFY=yes - shift # past argument - ;; - -c|--cursor) - CURSOR=yes - shift # past argument - ;; - -w|--wait) - shift - WAIT="$1" - if echo "$WAIT" | grep "[^0-9]" -q; then - echo "invalid value for wait '$WAIT'" >&2 - exit 3 - fi - shift - ;; - *) # unknown option - break # done with parsing --flags - ;; + -n | --notify) + NOTIFY=yes + shift # past argument + ;; + -c | --cursor) + CURSOR=yes + shift # past argument + ;; + -w | --wait) + shift + WAIT="$1" + if echo "$WAIT" | grep "[^0-9]" -q; then + echo "invalid value for wait '$WAIT'" >&2 + exit 3 + fi + shift + ;; + *) # unknown option + break # done with parsing --flags + ;; esac done @@ -54,7 +55,7 @@ ACTION=${1:-usage} SUBJECT=${2:-screen} FILE=${3:-$(getTargetDirectory)/$(date -Ins).png} -if [ "$ACTION" != "save" ] && [ "$ACTION" != "copy" ] && [ "$ACTION" != "savecopy" ] && [ "$ACTION" != "check" ]; then +printUsageMsg() { echo "Usage:" echo " grimshot [--notify] [--cursor] [--wait N] (copy|save) [active|screen|output|area|window|anything] [FILE|-]" echo " grimshot check" @@ -75,31 +76,34 @@ if [ "$ACTION" != "save" ] && [ "$ACTION" != "copy" ] && [ "$ACTION" != "savecop echo " window: Manually select a window." echo " anything: Manually select an area, window, or output." exit -fi +} notify() { notify-send -t 3000 -a grimshot "$@" } + notifyOk() { - [ "$NOTIFY" = "no" ] && return + notify_disabled='[ "$NOTIFY" = "no" ]' + action_involves_saving='[ "$ACTION" = "save" ] || [ "$ACTION" = "savecopy" ]' + + when "$notify_disabled" "return" TITLE=${2:-"Screenshot"} MESSAGE=${1:-"OK"} - if [ "$ACTION" = "save" ] || [ "$ACTION" = "savecopy" ]; then - notify "$TITLE" "$MESSAGE" -i "$FILE" - else - notify "$TITLE" "$MESSAGE" - fi + whenOtherwise "$action_involves_saving" \ + 'notify "$TITLE" "$MESSAGE" -i "$FILE"' \ + 'notify "$TITLE" "$MESSAGE"' } + notifyError() { - if [ $NOTIFY = "yes" ]; then - TITLE=${2:-"Screenshot"} - MESSAGE=${1:-"Error taking screenshot with grim"} - notify -u critical "$TITLE" "$MESSAGE" - else - echo "$1" - fi + notify_enabled='[ "$NOTIFY" = "yes" ]' + TITLE=${2:-"Screenshot"} + MESSAGE=${1:-"Error taking screenshot with grim"} + + whenOtherwise "$notify_enabled" \ + 'notify -u critical "$TITLE" "$MESSAGE"' \ + 'echo "$1"' } die() { @@ -110,11 +114,12 @@ die() { check() { COMMAND=$1 - if command -v "$COMMAND" > /dev/null 2>&1; then - RESULT="OK" - else - RESULT="NOT FOUND" - fi + command_exists='command -v "$COMMAND" > /dev/null 2>&1' + + whenOtherwise "$command_exists" \ + 'RESULT="OK"' \ + 'RESULT="NOT FOUND"' + echo " $COMMAND: $RESULT" } @@ -122,16 +127,20 @@ takeScreenshot() { FILE=$1 GEOM=$2 OUTPUT=$3 - if [ -n "$OUTPUT" ]; then - grim ${CURSOR:+-c} -o "$OUTPUT" "$FILE" || die "Unable to invoke grim" - elif [ -z "$GEOM" ]; then - grim ${CURSOR:+-c} "$FILE" || die "Unable to invoke grim" - else - grim ${CURSOR:+-c} -g "$GEOM" "$FILE" || die "Unable to invoke grim" - fi -} -if [ "$ACTION" = "check" ] ; then + output_provided='[ -n "$OUTPUT" ]' + geom_not_provided='[ -z "$GEOM" ]' + + output_action='grim ${CURSOR:+-c} -o "$OUTPUT" "$FILE" || die "Unable to invoke grim"' + full_screenshot_action='grim ${CURSOR:+-c} "$FILE" || die "Unable to invoke grim"' + geometry_screenshot_action='grim ${CURSOR:+-c} -g "$GEOM" "$FILE" || die "Unable to invoke grim"' + + any \ + "$output_provided:$output_action" \ + "$geom_not_provided:$full_screenshot_action" \ + "true:$geometry_screenshot_action" +} +checkRequiredTools() { echo "Checking if required tools are installed. If something is missing, install it to your system and make it available in PATH..." check grim check slurp @@ -140,61 +149,103 @@ if [ "$ACTION" = "check" ] ; then check jq check notify-send exit -elif [ "$SUBJECT" = "area" ] ; then +} + +selectArea() { GEOM=$(slurp -d) - # Check if user exited slurp without selecting the area - if [ -z "$GEOM" ]; then - exit 1 - fi + geomIsEmpty='[ -z "$GEOM" ]' + when "$geomIsEmpty" "exit 1" WHAT="Area" -elif [ "$SUBJECT" = "active" ] ; then +} + +selectActiveWindow() { FOCUSED=$(swaymsg -t get_tree | jq -r 'recurse(.nodes[]?, .floating_nodes[]?) | select(.focused)') GEOM=$(echo "$FOCUSED" | jq -r '.rect | "\(.x),\(.y) \(.width)x\(.height)"') APP_ID=$(echo "$FOCUSED" | jq -r '.app_id') WHAT="$APP_ID window" -elif [ "$SUBJECT" = "screen" ] ; then +} + +selectScreen() { GEOM="" WHAT="Screen" -elif [ "$SUBJECT" = "output" ] ; then +} + +selectOutput() { GEOM="" OUTPUT=$(swaymsg -t get_outputs | jq -r '.[] | select(.focused)' | jq -r '.name') WHAT="$OUTPUT" -elif [ "$SUBJECT" = "window" ] ; then +} + +selectWindow() { GEOM=$(swaymsg -t get_tree | jq -r '.. | select(.pid? and .visible?) | .rect | "\(.x),\(.y) \(.width)x\(.height)"' | slurp -r) - # Check if user exited slurp without selecting the area - if [ -z "$GEOM" ]; then - exit 1 - fi + geomIsEmpty='[ -z "$GEOM" ]' + when "$geomIsEmpty" "exit 1" WHAT="Window" -elif [ "$SUBJECT" = "anything" ] ; then +} + +selectAnything() { GEOM=$(swaymsg -t get_tree | jq -r '.. | select(.pid? and .visible?) | .rect | "\(.x),\(.y) \(.width)x\(.height)"' | slurp -o) - # Check if user exited slurp without selecting the area - if [ -z "$GEOM" ]; then - exit - fi + geomIsEmpty='[ -z "$GEOM" ]' + when "$geomIsEmpty" "exit 1" WHAT="Selection" -else - die "Unknown subject to take a screen shot from" "$SUBJECT" -fi +} +handleSaveCopy() { + wl-copy --type image/png <"$FILE" || die "Clipboard error" + MESSAGE="$MESSAGE and clipboard" +} -if [ "$WAIT" != "no" ]; then - sleep "$WAIT" -fi +handleScreenshotSuccess() { + TITLE="Screenshot of $SUBJECT" + MESSAGE=$(basename "$FILE") + isSaveCopy='[ "$ACTION" = "savecopy" ]' + when "$isSaveCopy" "handleSaveCopy" + notifyOk "$MESSAGE" "$TITLE" + echo "$FILE" +} + +handleScreenshotFailure() { + notifyError "Error taking screenshot with grim" +} -if [ "$ACTION" = "copy" ] ; then +handleCopy() { takeScreenshot - "$GEOM" "$OUTPUT" | wl-copy --type image/png || die "Clipboard error" notifyOk "$WHAT copied to clipboard" -else - if takeScreenshot "$FILE" "$GEOM" "$OUTPUT"; then - TITLE="Screenshot of $SUBJECT" - MESSAGE=$(basename "$FILE") - if [ "$ACTION" = "savecopy" ]; then - wl-copy --type image/png < "$FILE" || die "Clipboard error" - MESSAGE="$MESSAGE and clipboard" - fi - notifyOk "$MESSAGE" "$TITLE" - echo "$FILE" - else - notifyError "Error taking screenshot with grim" - fi -fi +} + +handleSave() { + screenshotTaken="takeScreenshot \"$FILE\" \"$GEOM\" \"$OUTPUT\"" + whenOtherwise "$screenshotTaken" \ + "handleScreenshotSuccess" \ + "handleScreenshotFailure" +} + +handleScreenshot() { + actionIsInvalid='[ "$ACTION" != "save" ] && [ "$ACTION" != "copy" ] && [ "$ACTION" != "savecopy" ] && [ "$ACTION" != "check" ]' + actionIsCheck='[ "$ACTION" = "check" ]' + subjectIsArea='[ "$SUBJECT" = "area" ]' + subjectIsActiveWindow='[ "$SUBJECT" = "active" ]' + subjectIsScreen='[ "$SUBJECT" = "screen" ]' + subjectIsOutput='[ "$SUBJECT" = "output" ]' + subjectIsWindow='[ "$SUBJECT" = "window" ]' + subjectIsAnything='[ "$SUBJECT" = "anything" ]' + + any \ + "$actionIsInvalid:printUsageMsg" \ + "$actionIsCheck:checkRequiredTools" \ + "$subjectIsArea:selectArea" \ + "$subjectIsActiveWindow:selectActiveWindow" \ + "$subjectIsScreen:selectScreen" \ + "$subjectIsOutput:selectOutput" \ + "$subjectIsWindow:selectWindow" \ + "$subjectIsAnything:selectAnything" || die "Unknown subject to take a screenshot from" "$SUBJECT" + + wait='[ "$WAIT" != "no" ]' + when "$wait" "sleep $WAIT" + + actionIsCopy='[ "$ACTION" = "copy" ]' + + whenOtherwise "$actionIsCopy" \ + "handleCopy" \ + "handleSave" +} +handleScreenshot From 656aac06cd796b6ff542c134bb1ddf21bafb805f Mon Sep 17 00:00:00 2001 From: Qubut Date: Thu, 12 Sep 2024 09:49:14 +0200 Subject: [PATCH 2/4] refactor: reorganize grim files and modularize argument parsing - Moved all grim related files to their own directory - Refactored argument parsing into the `parseArgs` function for improved clarity. - Added `handleUnknownSubject` to handle unknown subjects explicitly. --- .../functional-helpers | 0 grimshot => grimshot/grimshot | 80 ++++++++++--------- .../grimshot-completion.bash | 0 grimshot.1 => grimshot/grimshot.1 | 0 grimshot.1.scd => grimshot/grimshot.1.scd | 0 5 files changed, 44 insertions(+), 36 deletions(-) rename functional-helpers => grimshot/functional-helpers (100%) rename grimshot => grimshot/grimshot (88%) rename grimshot-completion.bash => grimshot/grimshot-completion.bash (100%) rename grimshot.1 => grimshot/grimshot.1 (100%) rename grimshot.1.scd => grimshot/grimshot.1.scd (100%) diff --git a/functional-helpers b/grimshot/functional-helpers similarity index 100% rename from functional-helpers rename to grimshot/functional-helpers diff --git a/grimshot b/grimshot/grimshot similarity index 88% rename from grimshot rename to grimshot/grimshot index 7621680..2cae1ab 100755 --- a/grimshot +++ b/grimshot/grimshot @@ -13,6 +13,10 @@ ## See `man 1 grimshot` or `grimshot usage` for further details. . ./functional-helpers +NOTIFY=no +CURSOR= +WAIT=no + getTargetDirectory() { test -f "${XDG_CONFIG_HOME:-$HOME/.config}/user-dirs.dirs" && . "${XDG_CONFIG_HOME:-$HOME/.config}/user-dirs.dirs" @@ -20,40 +24,39 @@ getTargetDirectory() { echo "${XDG_SCREENSHOTS_DIR:-${XDG_PICTURES_DIR:-$HOME}}" } -NOTIFY=no -CURSOR= -WAIT=no +parseArgs() { + while [ $# -gt 0 ]; do + key="$1" + + case $key in + -n | --notify) + NOTIFY=yes + shift # past argument + ;; + -c | --cursor) + CURSOR=yes + shift # past argument + ;; + -w | --wait) + shift + WAIT="$1" + if echo "$WAIT" | grep "[^0-9]" -q; then + echo "invalid value for wait '$WAIT'" >&2 + exit 3 + fi + shift + ;; + *) # unknown option + break # done with parsing --flags + ;; + esac + done + + ACTION=${1:-usage} + SUBJECT=${2:-screen} + FILE=${3:-$(getTargetDirectory)/$(date -Ins).png} -while [ $# -gt 0 ]; do - key="$1" - - case $key in - -n | --notify) - NOTIFY=yes - shift # past argument - ;; - -c | --cursor) - CURSOR=yes - shift # past argument - ;; - -w | --wait) - shift - WAIT="$1" - if echo "$WAIT" | grep "[^0-9]" -q; then - echo "invalid value for wait '$WAIT'" >&2 - exit 3 - fi - shift - ;; - *) # unknown option - break # done with parsing --flags - ;; - esac -done - -ACTION=${1:-usage} -SUBJECT=${2:-screen} -FILE=${3:-$(getTargetDirectory)/$(date -Ins).png} +} printUsageMsg() { echo "Usage:" @@ -218,7 +221,9 @@ handleSave() { "handleScreenshotSuccess" \ "handleScreenshotFailure" } - +handleUnknownSubject() { + die "Unknown subject to take a screenshot from" "$SUBJECT" +} handleScreenshot() { actionIsInvalid='[ "$ACTION" != "save" ] && [ "$ACTION" != "copy" ] && [ "$ACTION" != "savecopy" ] && [ "$ACTION" != "check" ]' actionIsCheck='[ "$ACTION" = "check" ]' @@ -228,7 +233,7 @@ handleScreenshot() { subjectIsOutput='[ "$SUBJECT" = "output" ]' subjectIsWindow='[ "$SUBJECT" = "window" ]' subjectIsAnything='[ "$SUBJECT" = "anything" ]' - + subjectIsUnknown=true any \ "$actionIsInvalid:printUsageMsg" \ "$actionIsCheck:checkRequiredTools" \ @@ -237,7 +242,8 @@ handleScreenshot() { "$subjectIsScreen:selectScreen" \ "$subjectIsOutput:selectOutput" \ "$subjectIsWindow:selectWindow" \ - "$subjectIsAnything:selectAnything" || die "Unknown subject to take a screenshot from" "$SUBJECT" + "$subjectIsAnything:selectAnything" \ + "$subjectIsUnknown:handleUnknownSubject" wait='[ "$WAIT" != "no" ]' when "$wait" "sleep $WAIT" @@ -248,4 +254,6 @@ handleScreenshot() { "handleCopy" \ "handleSave" } + +parseArgs "$@" handleScreenshot diff --git a/grimshot-completion.bash b/grimshot/grimshot-completion.bash similarity index 100% rename from grimshot-completion.bash rename to grimshot/grimshot-completion.bash diff --git a/grimshot.1 b/grimshot/grimshot.1 similarity index 100% rename from grimshot.1 rename to grimshot/grimshot.1 diff --git a/grimshot.1.scd b/grimshot/grimshot.1.scd similarity index 100% rename from grimshot.1.scd rename to grimshot/grimshot.1.scd From 8a9d0e42d2de3adadb9013d8656807b52f5bc4bf Mon Sep 17 00:00:00 2001 From: Qubut Date: Thu, 12 Sep 2024 11:00:49 +0200 Subject: [PATCH 3/4] fix(grim): handle positional arguments and error message in notifyError - Introduced `POSITIONAL_ARGS` to collect and reassign positional arguments using `set -- $POSITIONAL_ARGS` after flag parsing, ensuring correct positional argument handling. - Ensured flag parsing (`--notify`, `--cursor`, `--wait`) is processed before positional arguments. - Stored `$1` as `error_message` in `notifyError` to fix incorrect reference during condition evaluation. --- grimshot/grimshot | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/grimshot/grimshot b/grimshot/grimshot index 2cae1ab..78d216a 100755 --- a/grimshot/grimshot +++ b/grimshot/grimshot @@ -25,17 +25,17 @@ getTargetDirectory() { } parseArgs() { - while [ $# -gt 0 ]; do - key="$1" + POSITIONAL_ARGS="" - case $key in + while [ $# -gt 0 ]; do + case "$1" in -n | --notify) NOTIFY=yes - shift # past argument + shift ;; -c | --cursor) CURSOR=yes - shift # past argument + shift ;; -w | --wait) shift @@ -46,12 +46,14 @@ parseArgs() { fi shift ;; - *) # unknown option - break # done with parsing --flags + *) # Treat anything else as a positional argument + POSITIONAL_ARGS="$POSITIONAL_ARGS $1" # Add positional argument to the string + shift ;; esac done + set -- $POSITIONAL_ARGS # Re-assign positional arguments ACTION=${1:-usage} SUBJECT=${2:-screen} FILE=${3:-$(getTargetDirectory)/$(date -Ins).png} @@ -102,11 +104,12 @@ notifyOk() { notifyError() { notify_enabled='[ "$NOTIFY" = "yes" ]' TITLE=${2:-"Screenshot"} - MESSAGE=${1:-"Error taking screenshot with grim"} + errorMssg=$1 + MESSAGE=${errorMssg:-"Error taking screenshot with grim"} whenOtherwise "$notify_enabled" \ - 'notify -u critical "$TITLE" "$MESSAGE"' \ - 'echo "$1"' + 'notify "$TITLE" "$MESSAGE" -u critical' \ + 'echo "$errorMssg"' } die() { From 8bba26aed67db60df7067762673b3d828f01d187 Mon Sep 17 00:00:00 2001 From: Qubut Date: Thu, 24 Oct 2024 02:01:25 +0200 Subject: [PATCH 4/4] fix(grimshot): resolve missing functional-helpers file error (#40) - Fixed issue where the script failed to source the functional-helpers file. By including it's functionality in the script itself Closes #40. --- grimshot/functional-helpers | 33 --------------------------------- grimshot/grimshot | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 34 deletions(-) delete mode 100755 grimshot/functional-helpers diff --git a/grimshot/functional-helpers b/grimshot/functional-helpers deleted file mode 100755 index dcd2213..0000000 --- a/grimshot/functional-helpers +++ /dev/null @@ -1,33 +0,0 @@ -#! /bin/sh -when() { - condition=$1 - action=$2 - - if eval "$condition"; then - eval "$action" - fi -} - -whenOtherwise() { - condition=$1 - true_action=$2 - false_action=$3 - - if eval "$condition"; then - eval "$true_action" - else - eval "$false_action" - fi -} - -any() { - for tuple in "$@"; do - condition=$(echo "$tuple" | cut -d: -f1) - action=$(echo "$tuple" | cut -d: -f2-) - if eval "$condition"; then - eval "$action" - return 0 - fi - done - return 1 # No conditions matched -} diff --git a/grimshot/grimshot b/grimshot/grimshot index 78d216a..e0dd7b7 100755 --- a/grimshot/grimshot +++ b/grimshot/grimshot @@ -11,7 +11,40 @@ ## Those are needed to be installed, if unsure, run `grimshot check` ## ## See `man 1 grimshot` or `grimshot usage` for further details. -. ./functional-helpers + +when() { + condition=$1 + action=$2 + + if eval "$condition"; then + eval "$action" + fi +} + +whenOtherwise() { + condition=$1 + true_action=$2 + false_action=$3 + + if eval "$condition"; then + eval "$true_action" + else + eval "$false_action" + fi +} + +any() { + for tuple in "$@"; do + condition=$(echo "$tuple" | cut -d: -f1) + action=$(echo "$tuple" | cut -d: -f2-) + if eval "$condition"; then + eval "$action" + return 0 + fi + done + return 1 # No conditions matched +} + NOTIFY=no CURSOR=