On Mon, Mar 06, 2023 at 02:06:32PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
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@xxxxxxxxxx>
> ---
> 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
