feat: add shellcheck ci check #1179
Conversation
bin/git-obliterate
Outdated
| @@ -19,5 +19,5 @@ then | |||
| else | |||
| # don't quote $range so that we can forward multiple rev-list arguments | |||
| git filter-branch -f --index-filter "git rm -r --cached ""$file"" --ignore-unmatch" \ | |||
| --prune-empty --tag-name-filter cat -- $range | |||
| --prune-empty --tag-name-filter cat -- "$range" | |||
There was a problem hiding this comment.
range may contain multiple options, let's add a shellcheck disable.
There was a problem hiding this comment.
I'm not an expert here but converted range to array so that multiple values would be quoted correctly in (theoretical) case they'd contain IFS or glob characters. LMKWYT? Could also just shellcheck disable=SC2086. Thanks for quick feedback on these few items 😎
|
Let's merge main branch to make CI pass. |
| git filter-branch -f --index-filter "git rm -r --cached ""$file"" --ignore-unmatch" \ | ||
| --prune-empty --tag-name-filter cat -- $range | ||
| --prune-empty --tag-name-filter cat -- "${range[@]}" |
There was a problem hiding this comment.
Should be "${range[*]}"?
There was a problem hiding this comment.
This is what SC2086 doc indicated and looks correct based on the bash docs.
If the subscript is ‘@’ or ‘’, the word expands to all members of the array name. These subscripts differ only when the word appears within double quotes. If the word is double-quoted, ${name[]} expands to a single word with the value of each array member separated by the first character of the IFS variable, and ${name[@]} expands each element of name to a separate word.
And indeed that seems to be the case:
$ ranges=( "range 1" "range 2" "range 3" ); for range in "${ranges[*]}"; do echo "$range"; done
range 1 range 2 range 3
$ ranges=( "range 1" "range 2" "range 3" ); for range in "${ranges[@]}"; do echo "$range"; done
range 1
range 2
range 3
|
@oikarinen |
bin/git-guilt
Outdated
| @@ -44,10 +44,10 @@ for file in $(git diff --name-only "$@") | |||
| do | |||
| test -n "$DEBUG" && echo "git blame $file" | |||
| # $1 - since $2 - until | |||
| git blame $NOT_WHITESPACE --line-porcelain "$1" -- "$file" 2> /dev/null | | |||
| git blame "$NOT_WHITESPACE" --line-porcelain "$1" -- "$file" 2> /dev/null | | |||
There was a problem hiding this comment.
These new changes will cause git blame to fail whenever -w (or --ignore-whitespace) is not supplied at the command line.
bin/git-scp
Outdated
| _info "Pushing to $remote ($(git config "remote.$remote.url"))" && | ||
| rsync -rlDv --files-from="$_TMP" ./ "$(git config "remote.$remote.url")/" && | ||
| git add --force $list && | ||
| git add --force "$list" && |
There was a problem hiding this comment.
These new changes will now cause git add to fail in all cases (except the case where a single file is in list). Before, it would only fail on files that contained whitespace.
bin/git-scp
Outdated
| @@ -108,10 +108,10 @@ function scp_and_stage | |||
| then | |||
| local _TMP=${0///} | |||
| echo "$list" > "$_TMP" && | |||
| _sanitize $list && | |||
| _sanitize "$list" && | |||
There was a problem hiding this comment.
Same comment as below.
Thanks, if the test suite doesn't cover the changes probably best approach is to just ignore and perhaps have an issue(s) in backlog to fix those to limit the size of the PR. |
Sounds good to me! |
bin/git-scp
Outdated
| @@ -107,6 +107,7 @@ function scp_and_stage | |||
| if [ -n "$list" ] | |||
| then | |||
| local _TMP=${0///} | |||
| # shellcheck disable=SC2086 | |||
There was a problem hiding this comment.
Bad indent and bad location?
There was a problem hiding this comment.
Hey, tabs vs spaces thing. Sorry I rebased the branch to main instead of merge from old habits :(
This was mentioned in some older PR.
oikarinen
force-pushed
the
shellcheck
branch
from
ba0bb78 to
329b9f8
Compare
oikarinen
force-pushed
the
shellcheck
branch
from
329b9f8 to
b5e760e
Compare
|
Great, thanks! |
This was mentioned in some older PR. Autofixes in the same commit, some which seem actual bugs.