Re: [PATCH 2/8] ahead-behind: parse tip references
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Mar 7, 2023, 00:43 UTC
- Message-ID
- <ZAaIojLkjw3hHGm+@nand.local>
- In-Reply-To
- <08fc0710017d4b6178a8c5772e0f15fc69c84ad6.1678111599.git.gitgitgadget@gmail.com>
On Mon, Mar 06, 2023 at 02:06:32PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 9 quoted lines
> 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...
Show 70 quoted lines
> 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