On Sat, Nov 23 2024, Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > On Sat, Nov 23, 2024 at 12:01 AM Rasmus Villemoes > <linux@xxxxxxxxxxxxxxxxxx> wrote: >> v4: >> >> - Switch the logic to make use of the return values from try_tag, >> instead of asking whether $count has been set. > > > No, please do not do this. > > As I replied in v3, my plan is to set -e, because otherwise > the shell script is fragile. > > With this version, -e will not work in try_tag() > because it is used in the if condition. I'm confused. Previously you said that "either style is fine with me". Now you've invented some necessity to add "set -e", which of course yes, is then suppressed inside try_tag. But there is not a single statement within that that would cause "set -e" to exit anyway: The only one that is not a simple assignment or is itself a test is the "set -- $()", and git rev-list failing there does not cause set -e to trigger. Aside from that, I'm skeptical to introducing set -e anyway, it's simply too hard to reason about what it will actually do. http://mywiki.wooledge.org/BashFAQ/105 . But you're the maintainer. >> +try_tag() { >> + tag="$1" >> + >> + # Is $tag an annotated tag? >> + [ "$(git cat-file -t "$tag" 2> /dev/null)" = tag ] || return 1 >> + >> + # Is it an ancestor of HEAD, and if so, how many commits are in $tag..HEAD? >> + # shellcheck disable=SC2046 # word splitting is the point here >> + set -- $(git rev-list --count --left-right "$tag"...HEAD 2> /dev/null) >> + >> + # $1 is 0 if and only if $tag is an ancestor of HEAD. Use >> + # string comparison, because $1 is empty if the 'git rev-list' >> + # command somehow failed. >> + [ "$1" = 0 ] || return 1 >> + >> + # $2 is the number of commits in the range $tag..HEAD, possibly 0. >> + count="$2" > > Redundant double-quotes. Perhaps, but sorry, I'm not code-golfing, and trying to remember when quotes can be elided when variables are referenced is simply not something I spend my very limited brain capacity on. Feel free to make any adjustments you want and commit that, or drop this, I'm not sending a v5 as that seems to be a waste of everybody's time. Rasmus
