Mostly glaring problems found by shellcheck by nabijaczleweli · Pull Request #55 · carnager/teiler

nabijaczleweli · 2020-11-17T19:51:54Z

For the record, the original shellcheck logs: shellcheck.orig.gz, shellcheck_helper.orig.gz

I tried to retain the original semantics, if any, but to make them less dangerous/work better. Some warnings still remain (like the &&|| not being ifelse one). I would recommend going through them.

Current shellcheck logs:

In teiler line 16:
  source /etc/teiler/teiler.conf
  ^-- SC1091: Not following: /etc/teiler/teiler.conf was not specified as input (see shellcheck -x).


In teiler line 27:
export ext=${ext}; source "$HOME/.config/teiler/config"
                   ^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In teiler line 44:
  cd /etc/teiler/profiles
  ^-- SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.


In teiler line 55:
  if [[ ! -d "${img_path}" ]]; then
              ^-- SC2154: img_path is referenced but not assigned.


In teiler line 70:
  if [[ ! -d "${vid_path}" ]]; then
              ^-- SC2154: vid_path is referenced but not assigned.


In teiler line 85:
  if [[ ! -d "${paste_path}" ]]; then
              ^-- SC2154: paste_path is referenced but not assigned.


In teiler line 99:
source "$HOME/.config/teiler/profiles/${profile}"
^-- SC1090: Can't follow non-constant source. Use a directive to specify location.
                                      ^-- SC2154: profile is referenced but not assigned.


In teiler line 102:
save="$noupload"
      ^-- SC2154: noupload is referenced but not assigned.


In teiler line 114:
    HELP="<span color='$help_color'>${upload}: Upload | ${save}: No Upload</span>"
                       ^-- SC2154: help_color is referenced but not assigned.
                                    ^-- SC2154: upload is referenced but not assigned.


In teiler line 115:
    menu=$(echo -e "< Exit\n---\n$STATE_RECORDING" | _rofi -dmenu -u 2 -p "teiler > ")
                          ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                               ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 126:
    menu=$(echo -e "< Exit\n---\n1 [ Screenshots ]>\n2 [ Screencasts ]>\n3 [ History ]>\n---\n4 Pastebin Text from Clipboard" | _rofi -dmenu -p "teiler > ")
                          ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                               ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                   ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                                       ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                                                       ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                                                            ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 134:
        "4 Pastebin Text from Clipboard") export filename="${paste_filemask}" && clipCmd && teiler_helper --upload text "${filename}";;
                                                           ^-- SC2154: paste_filemask is referenced but not assigned.


In teiler line 148:
    export filename="${img_filemask}"
                     ^-- SC2154: img_filemask is referenced but not assigned.


In teiler line 149:
    menu=$(echo -e "< Return to Main Menu\n---\n1 Monitor\n2 Fullscreen\n3 Area" | _rofi -dmenu -mesg "${HELP}" -p "Screenshot > ")
                                         ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                              ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                         ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                                       ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 154:
        "3 Area") mode="area"; desc="Area";;
                               ^-- SC2034: desc appears unused. Verify use (or export if used externally).


In teiler line 171:
        if [[ "$always_ul" == "1" ]]; then
               ^-- SC2154: always_ul is referenced but not assigned.


In teiler line 194:
        menu=$(echo -e "< Exit\n---\n$STATE_RECORDING" | _rofi -dmenu -u 2 -kb-custom-1 "${upload}" -kb-custom-2 "${save}" -mesg "${HELP}" -p "teiler > ")
                              ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                   ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 210:
        menu=$(echo -e "< Return to Main Menu\n---\n1 Fullscreen\n2 Area" | _rofi -dmenu -p "Screencast > ")
                                             ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                  ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                                ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 215:
        if [[ $menu == "1 Fullscreen" ]]; then isRecording && stopRecording && sleep 2 || ffmpegCmd fullscreen;
                                                                            ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In teiler line 216:
        elif [[ $menu == "2 Area" ]]; then isRecording && stopRecording && sleep 2 || ffmpegCmd area;
                                                                        ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In teiler line 223:
    menu=$(echo -e "< Return to Main Menu\n---\n1 Images\n2 Videos" | _rofi -dmenu -p "Uploads > ")
                                         ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                              ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                        ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 233:
    HELP="<span color='$help_color'>${view}: View | ${historyupload}: Upload | ${edit}: Edit | ${clip}: Copy to Clipboard</span>"
                                    ^-- SC2154: view is referenced but not assigned.
                                                    ^-- SC2154: historyupload is referenced but not assigned.
                                                                               ^-- SC2154: edit is referenced but not assigned.
                                                                                               ^-- SC2154: clip is referenced but not assigned.


In teiler line 234:
    imagemenu=$(echo -e "< Return to Upload Menu\n---\n$(ls -1 -r)" | _rofi -dmenu -select "${entry}" -kb-custom-1 "${view}" -kb-custom-2 "${historyupload}" -kb-custom-3 "${edit}" -kb-custom-4 "${clip}" -mesg "${HELP}" -p "Choose > ")
                                                ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                     ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 240:
    elif [[ $val -eq 10 || $val -eq 0 ]]; then $viewer "${imagemenu}"; export entry="${imagemenu}"; imageMenu
                                               ^-- SC2154: viewer is referenced but not assigned.


In teiler line 243:
        $editor "${imagemenu2}-mod.png"
        ^-- SC2154: editor is referenced but not assigned.


In teiler line 256:
    videomenu=$(echo -e "< Return to Upload Menu\n---\n$(ls -1)" | _rofi -dmenu -select "${entry}" -kb-custom-1 "${view}" -kb-custom-2 "${historyupload}" -mesg "${HELP}" -p "Choose > ")
                                                ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                     ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 258:
    videomenu2="${videomenu%.*}"
    ^-- SC2034: videomenu2 appears unused. Verify use (or export if used externally).


In teiler line 261:
    elif [[ "$val" -eq 10 || "$val" -eq 0 ]]; then $player "${videomenu}"; export entry="${videomenu}"; videoMenu
                                                   ^-- SC2154: player is referenced but not assigned.


In teiler line 275:
    if [[ -z "$rate" ]]; then
              ^-- SC2154: rate is referenced but not assigned.


In teiler line 285:
    slopString=$(slop -b "$slop_border" -c "$slop_color")
                          ^-- SC2154: slop_border is referenced but not assigned.
                                            ^-- SC2154: slop_color is referenced but not assigned.


In teiler line 302:
    source "$HOME/.config/teiler/profiles/${profile}"
    ^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In teiler line 305:
    isRecording && { notify "$time" 'Screencast already in progress'; echo "Already recording Screen"; exit 1; }
                             ^-- SC2154: time is referenced but not assigned (for output from commands, use "$(time ...)" ).


In teiler line 324:
        encopts="$rect_encopts"
                 ^-- SC2154: rect_encopts is referenced but not assigned.


In teiler line 329:
    ffmpeg -f x11grab ${border} -s "$res" -i "$ffmpeg_display+$ffmpeg_offset" $ffaudio $encopts "${vid_path}/${filename}" &
                      ^-- SC2154: border is referenced but not assigned.
                      ^-- SC2086: Double quote to prevent globbing and word splitting.
                                                                              ^-- SC2154: ffaudio is referenced but not assigned.
                                                                              ^-- SC2086: Double quote to prevent globbing and word splitting.
                                                                                       ^-- SC2086: Double quote to prevent globbing and word splitting.


In teiler line 335:
    if [[ "$hidecursor" == "yes" ]]; then
           ^-- SC2154: hidecursor is referenced but not assigned.


In teiler line 352:
      if [[ $2 == "area" ]]; then maim $cursor ${rect_border} ${rect_color} -s -d "${delay}" "${img_path}/${3}";
                                               ^-- SC2086: Double quote to prevent globbing and word splitting.
                                                              ^-- SC2086: Double quote to prevent globbing and word splitting.


In teiler line 356:
        if [[ $2 == "area" ]]; then maim $cursor ${rect_border} ${rect_color} -s "${img_path}/${3}";
                                                 ^-- SC2086: Double quote to prevent globbing and word splitting.
                                                                ^-- SC2086: Double quote to prevent globbing and word splitting.


In teiler line 369:
        menu=$(echo -e "< Exit\n---\n1 Fullscreen\n2 Area" | _rofi -dmenu -p "> ")
                              ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                   ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".
                                                 ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 370:
        if [[ $menu == "1 Fullscreen" ]]; then isRecording && stopRecording && sleep 2 || ffmpegCmd fullscreen;
                                                                            ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In teiler line 371:
        elif [[ $menu == "2 Area" ]]; then isRecording && stopRecording && sleep 2 || ffmpegCmd area;
                                                                        ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In teiler line 377:
        ask="$(echo -e "1 Yes\n2 No" | _rofi -dmenu -p "Upload? > ")"
                             ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler line 432:
      if [[ $3 == "area" ]]; then isRecording && askPrompt && sleep 2 || ffmpegCmd area;
                                                           ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In teiler line 433:
      elif [[ $3 == "fullscreen" ]]; then isRecording && askPrompt && sleep 2 || ffmpegCmd fullscreen;
                                                                   ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.

and

In teiler_helper line 3:
source "$HOME/.config/teiler/config"
^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In teiler_helper line 4:
if [[ "$img_ul" == "s3" || "$img_ul" == "scp" ]]; then
       ^-- SC2154: img_ul is referenced but not assigned.


In teiler_helper line 5:
    source "$HOME/.config/teiler/uploader/$img_ul"
    ^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In teiler_helper line 7:
if [[ "$vid_ul" == "s3" || "$vid_ul" == "scp" ]]; then
       ^-- SC2154: vid_ul is referenced but not assigned.


In teiler_helper line 8:
    source "$HOME/.config/teiler/uploader/$vid_ul"
    ^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In teiler_helper line 10:
if [[ "$paste_ul" == "s3" || "$paste_ul" == "scp" ]]; then
       ^-- SC2154: paste_ul is referenced but not assigned.


In teiler_helper line 11:
    source "$HOME/.config/teiler/uploader/$paste_ul"
    ^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In teiler_helper line 19:
        cd "${img_path}" || return; fb $m "${@}"; x_clip; notify-send -a "teiler" "Image Uploaded" "$(xclip -o)"
            ^-- SC2154: img_path is referenced but not assigned.


In teiler_helper line 23:
    imageUpload () { for x in $files; do scpUpload "Image" "${img_path}" "$x" "${scp_host}" "${scp_path_img}" "${http_img}"; done; }
                                                                                             ^-- SC2154: scp_path_img is referenced but not assigned.
                                                                                                               ^-- SC2154: http_img is referenced but not assigned.


In teiler_helper line 27:
    imageUpload () { cd "${img_path}" || return; s3cmd --no-progress put "${1}" "s3://${s3_bucket}/${s3_path_img}/"; notify-send -a "teiler" "Image Uploaded" "${s3_http_img}/${1}"; echo -n "${s3_http_img}/${1}" | xclip; x_clip; }
                                                                                      ^-- SC2154: s3_bucket is referenced but not assigned.
                                                                                                   ^-- SC2154: s3_path_img is referenced but not assigned.
                                                                                                                                                               ^-- SC2154: s3_http_img is referenced but not assigned.


In teiler_helper line 35:
    videoUpload () { cd "${vid_path}" || return; fb "${1}"; x_clip; notify-send -a "teiler" "Video Uploaded" "$(xclip -o)"; }
                         ^-- SC2154: vid_path is referenced but not assigned.


In teiler_helper line 37:
    videoUpload () { scpUpload "Video" "${vid_path}" "$1" "${scp_host}" "${scp_path_vid}" "${http_vid}"; }
                                                                         ^-- SC2154: scp_path_vid is referenced but not assigned.
                                                                                           ^-- SC2154: http_vid is referenced but not assigned.


In teiler_helper line 39:
    videoUpload () { cd "${vid_path}" || return; s3cmd --no-progress put "${1}" "s3://${s3_bucket}/${s3_path_vid}/"; notify-send -a "teiler" "Video Uploaded" "${s3_http_vid}/${1}"; echo -n "${s3_http_vid}/${1}" | xclip; x_clip; }
                                                                                                   ^-- SC2154: s3_path_vid is referenced but not assigned.
                                                                                                                                                               ^-- SC2154: s3_http_vid is referenced but not assigned.


In teiler_helper line 46:
    clipUpload () { cd "${paste_path}" || return; fb "${1}"; x_clip; notify-send -a "teiler" "Paste Uploaded" "$(xclip -o)"; }
                        ^-- SC2154: paste_path is referenced but not assigned.


In teiler_helper line 48:
    clipUpload () { scpUpload "Paste" "${paste_path}" "$1" "${scp_host}" "${scp_path_paste}" "${http_paste}"; }
                                                                          ^-- SC2154: scp_path_paste is referenced but not assigned.
                                                                                              ^-- SC2154: http_paste is referenced but not assigned.


In teiler_helper line 50:
    clipUpload () { xclip -o 2>&1 | curl -F 'f:1=<-' ix.io | tr -d "\n" | xclip; x_clip; notify-send -a "teiler" "Paste Uploaded" "$(xclip -o)"; }
                                                                    ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In teiler_helper line 64:
    if [[ "$?" == 0 ]]; then
          ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

iFreilicht

I like it! Does the rewrite change functionality in any way?

Additionally, a change to the contributing guidelines would be nice, to tell people to run spellcheck before committing.

iFreilicht · 2021-02-22T11:58:04Z

teiler

        "< Return to Main Menu") mainMenu;;
      esac
-    if [[ $rofi_exit -eq 10 ]]; then
+    if [[ "$rofi_exit" -eq 10 ]]; then


Above, you replaced $paste_path with "${paste_path}". Why not here?

Dunno, likely because it has the ${} version just below it, I guess?

iFreilicht · 2021-02-22T11:59:45Z

teiler

+    ffmpeg_display="$DISPLAY"
+    ffmpeg -f x11grab ${border} -s "$res" -i "$ffmpeg_display+$ffmpeg_offset" $ffaudio $encopts "${vid_path}/${filename}" &
+    echo "$!" > "$SCREENCAST_PIDFILE"
+    notify-send -a "teiler" -t "1" "teiler" "Screencast started"


Would be nice to have all this simplification as a separate commit. Would make reviewing easier.

I no longer use teiler so I no longer have a way of usefully testing this, and I can't guarantee anything I do to this mess is correct by just looking at it, unfortunately.

teiler

nabijaczleweli · 2021-02-22T13:08:46Z

It should behave the same (save for some of the bits that exit early instead of operating in the wrong working directory).

As for guidelines, I don't see any, so I can't add to them.

iFreilicht · 2021-02-22T15:54:33Z

That's ok, I'll just make a mental note of that myself. You did run teiler with these changes successfully for a while, correct?

nabijaczleweli · 2021-02-22T16:13:11Z

Yeah, if memory serves

Fix glaring problems found by shellcheck. Simplify screencast code a bit

b801705

iFreilicht reviewed Feb 22, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mostly glaring problems found by shellcheck#55

Mostly glaring problems found by shellcheck#55
nabijaczleweli wants to merge 1 commit intocarnager:masterfrom
nabijaczleweli:shell-uh-check-i-guess

nabijaczleweli commented Nov 17, 2020

Uh oh!

iFreilicht left a comment •

edited

Loading

Uh oh!

iFreilicht Feb 22, 2021

Uh oh!

nabijaczleweli Feb 22, 2021

Uh oh!

iFreilicht Feb 22, 2021

Uh oh!

nabijaczleweli Feb 22, 2021

Uh oh!

Uh oh!

nabijaczleweli commented Feb 22, 2021

Uh oh!

iFreilicht commented Feb 22, 2021

Uh oh!

nabijaczleweli commented Feb 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

nabijaczleweli commented Nov 17, 2020

Uh oh!

iFreilicht left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iFreilicht Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

nabijaczleweli Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

iFreilicht Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

nabijaczleweli Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nabijaczleweli commented Feb 22, 2021

Uh oh!

iFreilicht commented Feb 22, 2021

Uh oh!

nabijaczleweli commented Feb 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

iFreilicht left a comment •

edited

Loading