Skip to content

Road to POSIX #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rgcv wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Road to POSIX #26

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 126 additions & 62 deletions notify-send.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ make_action() {

make_hint() {
type=$(convert_type "$1")
[[ ! $? = 0 ]] && return 1
[ $? -ne 0 ] && return 1
Copy link
Copy Markdown
Author

@rgcv rgcv Jan 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good place to start: conditionals in POSIX shell don't use double brackets, and the subset of things one is allowed to do is more limited as well. Instead, single brackets are used. Each pair of brackets is equivalent to the test(1p) command.

Here, specifically, we replace [[ ! ... = ... ]] with a numeric comparison [ ... -ne ... ] since we're testing the value of $?.

name="$2"
[[ "$type" = string ]] && command="\"$3\"" || command="$3"
[ "$type" = string ] && command="\"$3\"" || command="$3"
echo "\"$name\": <$type $command>"
}

Expand Down Expand Up @@ -117,14 +117,14 @@ notify() {
"${actions}" "${hints}" "int32 $EXPIRE_TIME" \
| parse_notification_id)

if [[ -n "$STORE_ID" ]] ; then
if [ -n "$STORE_ID" ] ; then
echo "$NOTIFICATION_ID" > "$STORE_ID"
fi
if [[ -n "$PRINT_ID" ]] ; then
if [ -n "$PRINT_ID" ] ; then
echo "$NOTIFICATION_ID"
fi

if [[ -n "$FORCE_EXPIRE" ]] ; then
if [ -n "$FORCE_EXPIRE" ] ; then
SLEEP_TIME="$( LC_NUMERIC=C printf %f "${EXPIRE_TIME}e-3" )"
( sleep "$SLEEP_TIME" ; notify_close "$NOTIFICATION_ID" ) &
fi
Expand Down Expand Up @@ -157,22 +157,22 @@ process_category() {

process_hint() {
IFS=: read type name command <<< "$1"
if [[ -z "$name" ]] || [[ -z "$command" ]] ; then
if [ -z "$name" ] || [ -z "$command" ] ; then
echo "Invalid hint syntax specified. Use TYPE:NAME:VALUE."
exit 1
fi
hint="$(make_hint "$type" "$name" "$command")"
if [[ ! $? = 0 ]] ; then
if [ $? -ne 0 ] ; then
echo "Invalid hint type \"$type\". Valid types are int, double, string and byte."
exit 1
fi
HINTS=("${HINTS[@]}" "$hint")
}

maybe_run_action_handler() {
if [[ -n "$NOTIFICATION_ID" ]] && [[ -n "$ACTION_COMMANDS" ]]; then
if [ -n "$NOTIFICATION_ID" ] && [ -n "$ACTION_COMMANDS" ]; then
local notify_action="$(dirname ${BASH_SOURCE[0]})/notify-action.sh"
if [[ -x "$notify_action" ]] ; then
if [ -x "$notify_action" ] ; then
"$notify_action" "$NOTIFICATION_ID" "${ACTION_COMMANDS[@]}" &
exit 0
else
Expand All @@ -184,7 +184,7 @@ maybe_run_action_handler() {

process_action() {
IFS=: read name command <<<"$1"
if [[ -z "$name" ]] || [[ -z "$command" ]]; then
if [ -z "$name" ] || [ -z "$command" ]; then
echo "Invalid action syntax specified. Use NAME:COMMAND."
exit 1
fi
Expand All @@ -200,109 +200,172 @@ process_special_action() {
action_key="$1"
command="$2"

if [[ -z "$action_key" ]] || [[ -z "$command" ]]; then
if [ -z "$action_key" ] || [ -z "$command" ]; then
echo "Command must not be empty"
exit 1
fi

ACTION_COMMANDS=("${ACTION_COMMANDS[@]}" "$action_key" "$command")

if [[ "$action_key" != close ]]; then
if [ "$action_key" != close ]; then
local action="$(make_action "$action_key" "$name")"
ACTIONS=("${ACTIONS[@]}" "$action")
fi
}

process_posargs() {
if [[ "$1" = -* ]] && ! [[ "$positional" = yes ]] ; then
echo "Unknown option $1"
exit 1
case "$1" in
-*) if [ "$positional" != yes ] ; then
echo "Unknown option $1"
exit 1
fi ;;
esac
Comment on lines -217 to +222
Copy link
Copy Markdown
Author

@rgcv rgcv Jan 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we can't really do regular expression matching with the regular test or [ ], we have to resort to other means to replace similar constructs. Fortunately, case provides some fairly basic matching capabilities that we can benefit from.

We first check if $1 is prefixed with - much in the same way as in the old if statement. If that is the case, we check if $positional is not yes, using a familiar != operator, meaning much the same as it does in other languages.

The difference between using = or != and using -eq, -ne, -gt, and co, is that the former apply only to strings while the latter are for numeric comparison.


if [ "$SUMMARY_SET" = n ]; then
SUMMARY="$1"
SUMMARY_SET=y
else
if [[ "$SUMMARY_SET" = n ]]; then
SUMMARY="$1"
SUMMARY_SET=y
else
BODY="$1"
fi
BODY="$1"
fi
}

while (( $# > 0 )) ; do
while [ $# -gt 0 ] ; do
Comment on lines -230 to +232
Copy link
Copy Markdown
Author

@rgcv rgcv Jan 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of bracket expansion does not exist in POSIX shell. The replacement is test's numeric comparison operators. In this case, -gt, for greater-than. An quasi-symmetric -lt exists, as well as x-than-or-equal variants too.

case "$1" in
-\?|--help)
help
exit 0
;;

-v|--version)
echo "${0##*/} $VERSION"
exit 0
;;
-u|--urgency|--urgency=*)
[[ "$1" = --urgency=* ]] && urgency="${1#*=}" || { shift; urgency="$1"; }
process_urgency "$urgency"

-u|--urgency)
shift
process_urgency "$1"
;;
--urgency=*)
process_urgency "${1#*=}"
;;
Comment on lines 240 to 250
Copy link
Copy Markdown
Author

@rgcv rgcv Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are a bit trickier to handle and, for some situations, the solution is clean. However, in cases where the handling is a bit more convoluted than just checking the --option=* variant, one would probably need at least one extra repeated case statement and it starts polluting the option parsing section.

My recommendation would be replacing that kind of handling and validation with functions, much like this process_urgency, and sweep away all of that mess away from here.

Copy link
Copy Markdown
Author

@rgcv rgcv Jan 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a more convoluted approach, but it works for any kind of --option=*-like option

while [ $# -gt 0 ]; do
  case "$1" in
    --[[:lower:]]*=*)
      __opt="${1%=*}"
      __val="${1#*=}"
      shift
      set -- ignored "$__opt" "$__val" "$@"
      unset __opt __val
      ;;
  # other options
  esac
shift
done

Let's break it up:

  1. The pattern --[[:lower:]]*=* is equivalent to the --[a-z].*=.* extended regular expression, though some character classes take LC_CTYPE into consideration. [:lower:] could be replaced with a-z:
    • Not the strongest of checks, but a decent one to start with. Further validation could incur after an initial match.
  2. We break the option and the value into temporary variables (__opt and __val resp.) for later usage;
  3. shift the now parsed option
  4. Use set -- to set positional argument values:
    • We set the first one ($1) to some mock value;
    • $2 and $3 become the option and the value, respectively;
    • The rest is set as whatever still needs to be processed;
  5. We clean up (unset) the temporary variables

What happens next is we break out of the case, the mock value we set gets shifted, and we are going to hit the case (if one exists) that covers option we parsed. The rest is business as usual.

Say the following arguments are passed in

notify-send.sh --option=value summary body

Here, $1 is --option=value, and $@ would be summary body. Once we parse $1, we get __opt=--option and __val=value. We shift, resulting in the following situation:

notify-send.sh summary body

However, we then reset the positional arguments, putting them in the following state:

notify-send.sh ignored --option value summary body

Then, after we clean up and break out, the ignored mock argument is shifted and we end up with:

notify-send.sh --option value summary body

🎉


-t|--expire-time|--expire-time=*)
[[ "$1" = --expire-time=* ]] && EXPIRE_TIME="${1#*=}" || { shift; EXPIRE_TIME="$1"; }
if ! [[ "$EXPIRE_TIME" =~ ^-?[0-9]+$ ]]; then
echo "Invalid expire time: ${EXPIRE_TIME}"
exit 1;
fi
case "$1" in
--expire-time=*) EXPIRE_TIME="${1#*=}" ;;
*) shift; EXPIRE_TIME="$1" ;;
esac
Comment on lines +253 to +256
Copy link
Copy Markdown
Author

@rgcv rgcv Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a prime example of what I just mentioned, where we need the extra case to check the different variant so we don't inadvertently shift arguments or repeat option parsing/validation logic in a different branch just for the --option=* variant.

# check if it is a numeric value
case "${EXPIRE_TIME#-}" in
*[![:digit:]]*|"")
echo "Invalid expire time: ${EXPIRE_TIME}"
exit 1
;;
esac
;;

-f|--force-expire)
FORCE_EXPIRE=yes
;;
-a|--app-name|--app-name=*)
[[ "$1" = --app-name=* ]] && APP_NAME="${1#*=}" || { shift; APP_NAME="$1"; }

-a|--app-name)
shift
APP_NAME="$1"
;;
-i|--icon|--icon=*)
[[ "$1" = --icon=* ]] && ICON="${1#*=}" || { shift; ICON="$1"; }
--app-name=*)
APP_NAME="${1#*=}"
;;
-c|--category|--category=*)
[[ "$1" = --category=* ]] && category="${1#*=}" || { shift; category="$1"; }
process_category "$category"

-i|--icon)
shift
ICON="$1"
;;
-h|--hint|--hint=*)
[[ "$1" = --hint=* ]] && hint="${1#*=}" || { shift; hint="$1"; }
process_hint "$hint"
--icon=*)
ICON="${1#*=}"
;;
-o | --action | --action=*)
[[ "$1" == --action=* ]] && action="${1#*=}" || { shift; action="$1"; }
process_action "$action"

-c|--category)
shift
process_category "$1"
;;
--category=*)
process_category "${1#*=}"
;;
-d | --default-action | --default-action=*)
[[ "$1" == --default-action=* ]] && default_action="${1#*=}" || { shift; default_action="$1"; }
process_special_action default "$default_action"

-h|--hint)
shift
process_hint "$1"
;;
-l | --close-action | --close-action=*)
[[ "$1" == --close-action=* ]] && close_action="${1#*=}" || { shift; close_action="$1"; }
process_special_action close "$close_action"
--hint=*)
process_hint "${1#*=}"
;;

-o|--action)
shift
process_action "$1"
;;
--action=*)
process_action "${1#*=}"
;;

-d|--default-action)
shift
process_special_action default "$1"
;;
--default-action=*)
process_special_action default "${1#*=}"
;;

-l|--close-action)
shift
process_special_action close "$1"
;;
--close-action=*)
process_special_action close "${1#*=}"
;;

-p|--print-id)
PRINT_ID=yes
;;
-r|--replace|--replace=*)
[[ "$1" = --replace=* ]] && REPLACE_ID="${1#*=}" || { shift; REPLACE_ID="$1"; }

-r|--replace)
shift
REPLACE_ID="$1"
;;
--replace=*)
REPLACE_ID="${1#*=}"
;;

-R|--replace-file|--replace-file=*)
[[ "$1" = --replace-file=* ]] && filename="${1#*=}" || { shift; filename="$1"; }
if [[ -s "$filename" ]]; then
case "$1" in
--replace-file=*) filename="${1#*=}" ;;
*) shift; filename="$1" ;;
esac
if [ -s "$filename" ]; then
REPLACE_ID="$(< "$filename")"
fi
STORE_ID="$filename"
;;

-s|--close|--close=*)
[[ "$1" = --close=* ]] && close_id="${1#*=}" || { shift; close_id="$1"; }
case "$1" in
--close=*) close_id="${1#*=}" ;;
*) shift; close_id="$1" ;;
esac
# always check that --close provides a numeric value
if [[ -z "$close_id" || ! "$close_id" =~ ^[0-9]+$ ]]; then
echo "Invalid close id: '$close_id'"
exit 1
fi
case "${close_id#*-}" in
*[![:digit:]]*|"")
echo "Invalid close id: '$close_id'"
exit 1
;;
esac
notify_close "$close_id"
exit $?
;;

--)
positional=yes
;;

*)
process_posargs "$1"
;;
Expand All @@ -311,14 +374,15 @@ while (( $# > 0 )) ; do
done

# always force --replace and --replace-file to provide a numeric value; 0 means no id provided
if [[ -z "$REPLACE_ID" || ! "$REPLACE_ID" =~ ^[0-9]+$ ]]; then
REPLACE_ID=0
fi
case "$REPLACE_ID" in
# empty or something in [[:digit:]]
""|*[![:digit:]]*) REPLACE_ID=0 ;;
esac

# urgency is always set
HINTS=("$(make_hint byte urgency "$URGENCY")" "${HINTS[@]}")

if [[ "$SUMMARY_SET" = n ]] ; then
if [ "$SUMMARY_SET" = n ] ; then
help
exit 1
else
Expand Down