Addtional fixes for `shellcheck`

akinomyoga commented 2 years ago

9c6e5d7 Maybe due to the difference of the shellcheck versions, but shellcheck-0.7.2 in my environment reports SC2034 also for this line.
84ef2b1 disable=SC2034 has been applied to __bp_last_ret_value in addition to BP_PIPESTATUS. It would be better to apply it only for the intended variable, BP_PIPESTATUS.
fffccd2 I noticed that return "${1-}" results in return '' and causes an error when $1 is empty. This is a regression of #135. Instead, we should specify the argument as ${1:+"$1"}.
cb8048a This is also subject to word splitting by unexpected IFS. We want to quote it as well.
16e09f2 I think it is cleaner and readable for beginners to follow the suggestion from shellcheck rather than ignoring the warning

akinomyoga commented 2 years ago

OK, I'll drop 84ef2b186f79ee0bea52fa99656314ca1255d52f.

rcaloras commented 2 years ago

LGTM 👍

rcaloras commented 2 years ago

Thank you for the PRs @akinomyoga!

akinomyoga commented 2 years ago

Thanks for reviewing!

rcaloras / bash-preexec