]> git.ipfire.org Git - thirdparty/git.git/commit - wt-status.c
diff: drop useless return from run_diff_{files,index} functions
authorJeff King <peff@peff.net>
Mon, 21 Aug 2023 20:18:55 +0000 (16:18 -0400)
committerJunio C Hamano <gitster@pobox.com>
Mon, 21 Aug 2023 22:33:24 +0000 (15:33 -0700)
commit25bd3acd048152dcf2de5f446d2bd21b5fb42b09
tree646d23f518b83a13e96ac1381816a4e11aeedd4f
parent3755077b506356017d942dbcc2ffe9cb17ecc606
diff: drop useless return from run_diff_{files,index} functions

Neither of these functions ever returns a value other than zero.
Instead, they expect unrecoverable errors to exit immediately, and
things like "--exit-code" are stored inside the diff_options struct to
be handled later via diff_result_code().

Some callers do check the return values, but many don't bother. Let's
drop the useless return values, which are misleading callers about how
the functions work. This could be seen as a step in the wrong direction,
as we might want to eventually "lib-ify" these to more cleanly return
errors up the stack, in which case we'd have to add the return values
back in. But there are some benefits to doing this now:

  1. In the current code, somebody could accidentally add a "return -1"
     to one of the functions, which would be erroneously ignored by many
     callers. By removing the return code, the compiler can notice the
     mismatch and force the developer to decide what to do.

     Obviously the other option here is that we could start consistently
     checking the error code in every caller. But it would be dead code,
     and we wouldn't get any compile-time help in catching new cases.

  2. It communicates the situation to callers, who may want to choose a
     different function. These functions are really thin wrappers for
     doing git-diff-files and git-diff-index within the process. But
     callers who care about recovering from an error here are probably
     better off using the underlying library functions, many of
     which do return errors.

If somebody eventually wants to teach these functions to propagate
errors, they'll have to switch back to returning a value, effectively
reverting this patch. But at least then they will be starting with a
level playing field: they know that they will need to inspect each
caller to see how it should handle the error.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/add.c
builtin/describe.c
builtin/diff-files.c
builtin/diff-index.c
builtin/diff.c
builtin/stash.c
builtin/submodule--helper.c
diff-lib.c
diff.h
wt-status.c