-
Notifications
You must be signed in to change notification settings - Fork 40
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.
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
Road to POSIX #26
base: master
Are you sure you want to change the base?
Road to POSIX #26
Conversation
@@ -78,9 +78,9 @@ make_action() { | |||
|
|||
make_hint() { | |||
type=$(convert_type "$1") | |||
[[ ! $? = 0 ]] && return 1 | |||
[ $? -ne 0 ] && return 1 |
There was a problem hiding this comment.
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 $?
.
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 |
There was a problem hiding this comment.
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.
while (( $# > 0 )) ; do | ||
while [ $# -gt 0 ] ; do |
There was a problem hiding this comment.
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.
-u|--urgency|--urgency=*) | ||
[[ "$1" = --urgency=* ]] && urgency="${1#*=}" || { shift; urgency="$1"; } | ||
process_urgency "$urgency" | ||
|
||
-u|--urgency) | ||
shift | ||
process_urgency "$1" | ||
;; | ||
--urgency=*) | ||
process_urgency "${1#*=}" | ||
;; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- The pattern
--[[:lower:]]*=*
is equivalent to the--[a-z].*=.*
extended regular expression, though some character classes takeLC_CTYPE
into consideration.[:lower:]
could be replaced witha-z
:- Not the strongest of checks, but a decent one to start with. Further validation could incur after an initial match.
- We break the option and the value into temporary variables (
__opt
and__val
resp.) for later usage; shift
the now parsed option- 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;
- We set the first one (
- 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
🎉
notify-send.sh
Outdated
case "${EXPIRE_TIME#-*}" in | ||
*[![:digit:]]*|"") | ||
echo "Invalid expire time: ${EXPIRE_TIME}" | ||
exit 1 | ||
;; | ||
esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once more, since we don't have extended (or even basic, it's just a subset) regular expression matching in POSIX shell, we must make due with what we have.
We first strip a potential leading -
minus sign from the candidate numeric value. Then, we test if there it is either empty or if there is a character present that isn't !
in the [:digit:]
(could be replaced with [0-9]
, honestly) character class (not as convenient as \d
, but gets the job done). In case we find an intruder, we report the error. So instead of checking against a numeric pattern inclusively, we invert the check. Works beautifuly 👍
case "$1" in | ||
--expire-time=*) EXPIRE_TIME="${1#*=}" ;; | ||
*) shift; EXPIRE_TIME="$1" ;; | ||
esac |
There was a problem hiding this comment.
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.
Oh, as an added note, instead of using edit: in fact, just a bit of exploration lead me to an interesting and clearer alternative for matching numeric values using expr "$variable" : '-\{,1\}[[:digit:]]\{1,\}$' One can already notice the anchor
The pattern, broken down: - # the '-' literal
\{,1\} # at most once (= <obj>?)
[[:digit:]] # decimal digit character class
\{1,\} # at least once (= <obj>+ or <obj><obj>*)
$ # end anchor which is equivalent to the expression already used in the script ( Here are a quick couple of (paranoia satiation) tests; $ pat='-\{,1\}[[:digit:]]\{1,\}$'
$ expr '1234' : "$pat"
4 # number of characters matched
$ expr '-1234' : "$pat"
5
$ expr ' 1234' : "$pat"
0
$ expr 'aaa1234' : "$pat"
0
$ expr '1234aaa' : "$pat"
0
$ expr '1234 ' : "$pat"
0
$ expr '-1234 ' : "$pat"
0
$ expr '-12aa34' : "$pat"
0 |
Here's an example pitting case ${variable#-} in
*[![:digit:]]*) echo "not ok"; exit 1 ;;
esac
# versus
if expr "$variable" : '-\{,1\}[[:digit:]]\{1,\}' = 0; then
echo "not ok"
exit 1
fi It's a matter of wanting to do it using pure shell or resorting to |
bd4721e
to
bef907e
Compare
bef907e
to
183bd4d
Compare
hmmm, at a glance it looks good, tho for some of the still hard regex stuff what do you think about using some awk magic? iirc the one true awk that should be standar for POSIX supports a rather nice subset of regex so it can be abused on functions to pass it some input and then pour awk's output into other functions and vars. |
Howdy 👋
Been a hot minute. I've returned with a scratch at the surface towards converting the
notify-send.sh
script into a POSIX shell-compliant one. This first batch of changes covers conditional constructs.BASH is quite comfortable and offers a series of things POSIX shell does not, such as convenient extended regular expression matching. POSIX shell is fairly poorer in this regard, but functionally equivalent alternatives exist. Here is my first proposal. It is a relatively substantial one, but I thought it would be best to break things into what I consider feature-set disparities between BASH and POSIX shell.
I'm creating a draft PR so this can be further discussed before it can be promoted to a proper PR and be more thoroughly tested as well.