| Message ID | 08fc0710017d4b6178a8c5772e0f15fc69c84ad6.1678111599.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ahead-behind: new builtin for counting multiple commit ranges | expand |
On Mon, Mar 06, 2023 at 02:06:32PM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <derrickstolee@github.com> > > Before implementing the logic to compute the ahead/behind counts, parse > the unknown options as commits and place them in a string_list. > > Be sure to error out when the reference is not found. > > Co-authored-by: Taylor Blau <me@ttaylorr.com> > Signed-off-by: Taylor Blau <me@ttaylorr.com> This all looks reasonable (and forging my S-o-b here and elsewhere throughout this series is all fine). I have seen most of this code before at least in its final state, but the intermediate bits are new to me. And they all look fine and familiar, except... > Signed-off-by: Derrick Stolee <derrickstolee@github.com> > --- > builtin/ahead-behind.c | 39 +++++++++++++++++++++++++++++++++++++++ > t/t4218-ahead-behind.sh | 10 ++++++++++ > 2 files changed, 49 insertions(+) > > diff --git a/builtin/ahead-behind.c b/builtin/ahead-behind.c > index a56cc565def..c1212cc8d46 100644 > --- a/builtin/ahead-behind.c > +++ b/builtin/ahead-behind.c > @@ -1,16 +1,31 @@ > #include "builtin.h" > #include "parse-options.h" > #include "config.h" > +#include "commit.h" > > static const char * const ahead_behind_usage[] = { > N_("git ahead-behind --base=<ref> [ --stdin | <revs> ]"), > NULL > }; > > +static int handle_arg(struct string_list *tips, const char *arg) > +{ > + struct string_list_item *item; > + struct commit *c = lookup_commit_reference_by_name(arg); > + > + if (!c) > + return error(_("could not resolve '%s'"), arg); > + > + item = string_list_append(tips, arg); > + item->util = c; > + return 0; > +} > + > int cmd_ahead_behind(int argc, const char **argv, const char *prefix) > { > const char *base_ref = NULL; > int from_stdin = 0; > + struct string_list tips = STRING_LIST_INIT_DUP; > > struct option ahead_behind_opts[] = { > OPT_STRING('b', "base", &base_ref, N_("base"), N_("base reference to process")), > @@ -26,5 +41,29 @@ int cmd_ahead_behind(int argc, const char **argv, const char *prefix) > > git_config(git_default_config, NULL); > > + if (from_stdin) { > + struct strbuf line = STRBUF_INIT; > + > + while (strbuf_getline(&line, stdin) != EOF) { > + if (!line.len) > + break; > + > + if (handle_arg(&tips, line.buf)) > + return 1; > + } > + > + strbuf_release(&line); > + } else { > + int i; > + for (i = 0; i < argc; ++i) { > + if (handle_arg(&tips, argv[i])) > + return 1; > + } > + } > + > + /* Early return for no tips. */ > + if (!tips.nr) > + return 0; > + ...are we missing a call to `string_list_clear()` here on `&tips`? Thanks, Taylor
diff --git a/builtin/ahead-behind.c b/builtin/ahead-behind.c index a56cc565def..c1212cc8d46 100644 --- a/builtin/ahead-behind.c +++ b/builtin/ahead-behind.c @@ -1,16 +1,31 @@ #include "builtin.h" #include "parse-options.h" #include "config.h" +#include "commit.h" static const char * const ahead_behind_usage[] = { N_("git ahead-behind --base=<ref> [ --stdin | <revs> ]"), NULL }; +static int handle_arg(struct string_list *tips, const char *arg) +{ + struct string_list_item *item; + struct commit *c = lookup_commit_reference_by_name(arg); + + if (!c) + return error(_("could not resolve '%s'"), arg); + + item = string_list_append(tips, arg); + item->util = c; + return 0; +} + int cmd_ahead_behind(int argc, const char **argv, const char *prefix) { const char *base_ref = NULL; int from_stdin = 0; + struct string_list tips = STRING_LIST_INIT_DUP; struct option ahead_behind_opts[] = { OPT_STRING('b', "base", &base_ref, N_("base"), N_("base reference to process")), @@ -26,5 +41,29 @@ int cmd_ahead_behind(int argc, const char **argv, const char *prefix) git_config(git_default_config, NULL); + if (from_stdin) { + struct strbuf line = STRBUF_INIT; + + while (strbuf_getline(&line, stdin) != EOF) { + if (!line.len) + break; + + if (handle_arg(&tips, line.buf)) + return 1; + } + + strbuf_release(&line); + } else { + int i; + for (i = 0; i < argc; ++i) { + if (handle_arg(&tips, argv[i])) + return 1; + } + } + + /* Early return for no tips. */ + if (!tips.nr) + return 0; + return 0; } diff --git a/t/t4218-ahead-behind.sh b/t/t4218-ahead-behind.sh index bc08f1207a0..3b8b9dc9887 100755 --- a/t/t4218-ahead-behind.sh +++ b/t/t4218-ahead-behind.sh @@ -14,4 +14,14 @@ test_expect_success 'git ahead-behind without --base' ' grep "usage:" err ' +test_expect_success 'git ahead-behind with broken tip' ' + test_must_fail git ahead-behind --base=HEAD bogus 2>err && + grep "could not resolve '\''bogus'\''" err +' + +test_expect_success 'git ahead-behind without tips' ' + git ahead-behind --base=HEAD 2>err && + test_must_be_empty err +' + test_done