Shellchecked xdg-open

* Added a .shellcheckrc file
* Replace legacy backticks with `"$(`
* Comment where `read -r` is inteletially placed and where the `-r` is omitted.
* Add quotes where they don't hurt
* fix iterating over user and system_dirs by using IFS properly in `open_generic_xdg_mime`
* Moved some `local` declarations out of the way
* Remove unused variable from `open_envvar`
* Disable checks fir the scary looking part of `open_envvar`
* Removed unneccessary variable in `open_flatpak`
*

Did *not* fix:
* `local` is posix "incompatible"

2 files changed, 42 insertions, 19 deletions

diff --git a/.shellcheckrc b/.shellcheckrc
new file mode 100644
index 0000000..d16c2b0
--- /dev/null
+++ b/.shellcheckrc
@@ -0,0 +1,9 @@
+
+# checking success after running a command
+# is a common practice trhought the xdg-utils
+# and it works well enough to not change it.
+disable=SC2181
+
+# x prefixes aren't pretty anymore,
+# but they don't hurt either.
+disable=SC2268
diff --git a/scripts/xdg-open.in b/scripts/xdg-open.in
index 02953fa..1373ca1 100755
--- a/scripts/xdg-open.in
+++ b/scripts/xdg-open.in
@@ -32,6 +32,8 @@ _USAGE
 # This handles backslashes but not quote marks.
 last_word()
 {
+    # Backslash handling is intended, not using `first` too
+    # shellcheck disable=SC2162,SC2034
     read first rest
     echo "$rest"
 }
@@ -47,7 +49,8 @@ get_key()
 
     IFS_="${IFS}"
     IFS=""
-    while read line
+    # No backslash handling here, first_word and last_word do that
+    while read -r line
     do
         case "$line" in
             "[Desktop Entry]")
@@ -148,7 +151,7 @@ open_kde()
           kde-open "$1"
         ;;
         5)
-          kde-open${KDE_SESSION_VERSION} "$1"
+          "kde-open${KDE_SESSION_VERSION}" "$1"
         ;;
         6)
           kde-open "$1"
@@ -273,7 +276,8 @@ open_enlightenment()
 open_flatpak()
 {
     if is_file_url_or_path "$1"; then
-        local file="$(file_url_to_path "$1")"
+        local file
+        file="$(file_url_to_path "$1")"
 
         check_input_file "$file"
 
@@ -284,14 +288,14 @@ open_flatpak()
             --timeout 5 \
             "" "3" {} 3< "$file"
     else
-        local uri="$1"
+        # $1 contains an URI
 
         gdbus call --session \
             --dest org.freedesktop.portal.Desktop \
             --object-path /org/freedesktop/portal/desktop \
             --method org.freedesktop.portal.OpenURI.OpenURI \
             --timeout 5 \
-            "" "$uri" {}
+            "" "$1" {}
     fi
 
     if [ $? -eq 0 ]; then
@@ -316,8 +320,8 @@ search_desktop_file()
     # look for both vendor-app.desktop, vendor/app.desktop
     if [ -r "$dir/$default" ]; then
       file="$dir/$default"
-    elif [ -r "$dir/`echo $default | sed -e 's|-|/|'`" ]; then
-      file="$dir/`echo $default | sed -e 's|-|/|'`"
+    elif [ -r "$dir/$(echo "$default" | sed -e 's|-|/|')" ]; then
+      file="$dir/$(echo "$default" | sed -e 's|-|/|')"
     fi
 
     if [ -r "$file" ] ; then
@@ -327,6 +331,7 @@ search_desktop_file()
             # FIXME: Actually LC_MESSAGES should be used as described in
             # http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s04.html
             localised_name="$(get_key "${file}" "Name")"
+            #shellcheck disable=SC2046 # Splitting is intentional here
             set -- $(get_key "${file}" "Exec" | last_word)
             # We need to replace any occurrence of "%f", "%F" and
             # the like by the target file. We examine each
@@ -370,7 +375,7 @@ search_desktop_file()
                         set -- "$@" "$arg"
                         ;;
                 esac
-                args=$(( $args - 1 ))
+                args=$(( args - 1 ))
             done
             [ $replaced -eq 1 ] || set -- "$@" "${target:-target_uri}"
             env "$command" "$@"
@@ -387,7 +392,7 @@ search_desktop_file()
 open_generic_xdg_mime()
 {
     filetype="$2"
-    default=`xdg-mime query default "$filetype"`
+    default="$(xdg-mime query default "$filetype")"
     if [ -n "$default" ] ; then
         xdg_user_dir="$XDG_DATA_HOME"
         [ -n "$xdg_user_dir" ] || xdg_user_dir="$HOME/.local/share"
@@ -395,8 +400,12 @@ open_generic_xdg_mime()
         xdg_system_dirs="$XDG_DATA_DIRS"
         [ -n "$xdg_system_dirs" ] || xdg_system_dirs=/usr/local/share/:/usr/share/
 
-DEBUG 3 "$xdg_user_dir:$xdg_system_dirs"
-        for x in `echo "$xdg_user_dir:$xdg_system_dirs" | sed 's/:/ /g'`; do
+        search_dirs="$xdg_user_dir:$xdg_system_dirs"
+        DEBUG 3 "$search_dirs"
+        old_ifs="$IFS"
+        IFS=:
+        for x in $search_dirs ; do
+            IFS="$old_ifs"
             search_desktop_file "$default" "$x/applications/" "$1" "$3"
         done
     fi
@@ -404,7 +413,7 @@ DEBUG 3 "$xdg_user_dir:$xdg_system_dirs"
 
 open_generic_xdg_x_scheme_handler()
 {
-    scheme="`echo $1 | LC_ALL=C sed -n 's/\(^[[:alpha:]][[:alnum:]+\.-]*\):.*$/\1/p'`"
+    scheme="$(echo "$1" | LC_ALL=C sed -n 's/\(^[[:alpha:]][[:alnum:]+\.-]*\):.*$/\1/p')"
     if [ -n "$scheme" ]; then
         filetype="x-scheme-handler/$scheme"
         open_generic_xdg_mime "" "$filetype" "$1"
@@ -419,7 +428,7 @@ has_single_argument()
 open_envvar()
 {
     local oldifs="$IFS"
-    local browser browser_with_arg
+    local browser
 
     IFS=":"
     for browser in $BROWSER; do
@@ -433,6 +442,8 @@ open_envvar()
             # Avoid argument injection.
             # See https://bugs.freedesktop.org/show_bug.cgi?id=103807
             # URIs don't have IFS characters spaces anyway.
+            # shellcheck disable=SC2086,SC2091,SC2059
+            # All the scary things here are intentional
             has_single_argument $1 && $(printf "$browser" "$1")
         else
             $browser "$1"
@@ -446,10 +457,11 @@ open_envvar()
 
 open_wsl()
 {
+    local win_path
     if has_url_scheme "$1"; then
-        local win_path="$1"
+        win_path="$1"
     else
-        local win_path="$(wslpath -aw "$1")"
+        win_path="$(wslpath -aw "$1")"
         [ $? -eq 0 ] || exit_failure_operation_failed
     fi
     explorer.exe "${win_path}"
@@ -464,12 +476,13 @@ open_wsl()
 open_generic()
 {
     if is_file_url_or_path "$1"; then
-        local file="$(file_url_to_path "$1")"
+        local file
+        file="$(file_url_to_path "$1")"
 
         check_input_file "$file"
 
         if has_display; then
-            filetype=`xdg-mime query filetype "$file" | sed "s/;.*//"`
+            filetype="$(xdg-mime query filetype "$file" | sed "s/;.*//")"
             # passing a path a url is okay too,
             # see desktop file specification for '%u'
             open_generic_xdg_mime "$file" "$filetype" "$1"
@@ -516,7 +529,8 @@ open_lxde()
 
     # pcmanfm only knows how to handle file:// urls and filepaths, it seems.
     if pcmanfm --help >/dev/null 2>&1 && is_file_url_or_path "$1"; then
-        local file="$(file_url_to_path "$1")"
+        local file
+        file="$(file_url_to_path "$1")"
 
         # handle relative paths
         if ! echo "$file" | grep -q ^/; then
@@ -586,7 +600,7 @@ DEBUG 2 "Selected DE $DE"
 # sanitize BROWSER (avoid calling ourselves in particular)
 case "${BROWSER}" in
     *:"xdg-open"|"xdg-open":*)
-        BROWSER="$(echo $BROWSER | sed -e 's|:xdg-open||g' -e 's|xdg-open:||g')"
+        BROWSER="$(echo "$BROWSER" | sed -e 's|:xdg-open||g' -e 's|xdg-open:||g')"
         ;;
     "xdg-open")
         BROWSER=