Rename git::is_dirty — it reads as a predicate but returns a string

[MihaiBojin]

git::is_dirty is named like a boolean predicate but is actually a
string-producing helper. It always exits 0, and signals its result by
printing -dirty or the empty string.

src/git.bash:27:

# Checks if the current Git working directory contains uncommitted changes.
#
# Prints nothing if the working directory is clean, '-dirty' otherwise.
function git::is_dirty() {
    if [ -n "$(git status --porcelain)" ]; then
        echo "-dirty"
    else
        echo ""
    fi
}

Why it matters

The is_ prefix is a strong convention for something usable as a condition, so
the natural call is:

if rt git::is_dirty; then
    echo "working tree is dirty"   # always runs — clean or not
fi

That branch is taken unconditionally, because the function exits 0 either way.
The failure is silent and reads as correct, which is the worst combination — a
release script gated on it would happily proceed, or refuse to, regardless of
the actual state.

I hit this writing a pre-flight check before cutting a release:

$ rt git::is_dirty && echo "DIRTY" || echo "clean"

  DIRTY          # working tree was clean

Current usage

There is exactly one internal call site, and it is correct — git::head_sha
uses it as a string suffix (src/git.bash:43):

echo "${git_sha}$(git::is_dirty)"

So this is not a live bug in the CLI. The exposure is that git::is_dirty is
also reachable as a public command (rt git::is_dirty), where the name invites
the predicate reading.

Suggestions

Either direction works; they have different compatibility costs.

Rename to reflect what it returns — e.g. git::dirty_suffix,
git::dirty_marker. Honest about being a string, no behaviour change. Breaking
for anyone calling the old name, so it may warrant keeping git::is_dirty as a
deprecated alias for a release.

Make it a real predicate — return 1 when clean, 0 when dirty, print
nothing. Matches the name, but changes head_sha: under set -e the
$(git::is_dirty) substitution in an assignment context would abort on a clean
tree, so that call site needs restructuring at the same time. It also breaks any
external caller relying on the printed suffix.

A third option, if the name should stay: document the string return prominently
and add git::is_dirty_p (or similar) as the predicate form.

Happy to send a PR for whichever you prefer.

[MihaiBojin]

Filed #31 for a few more of these in base.bash and main.sh, and referenced this one there. Same shape: a function whose name or exit code implies one contract while the behaviour is another.

The bit worth flagging here is where the is_dirty naming actually led. The website docs documented rt git::is_clean as the clean-tree check, which has never existed, and it survived until the docs were diffed against the dispatch table. Corrected in releasetools/website#3 and #4.

If someone takes #31, this is probably worth doing in the same pass.