Re: [PATCH v3 2/2] reflog: implement subcommand to drop reflogs

From: Christian Couder <christian.couder@gmail.com>
Date: Mar 18, 2025, 14:01 UTC
Message-ID: <CAP8UFD2+xQQbUevHTQangg__dq2M615pHLhYYUf7b9shpShD9Q@mail.gmail.com>
In-Reply-To: <20250314-493-add-command-to-purge-reflog-entries-v3-2-c24e23a6146d@gmail.com>

On Fri, Mar 14, 2025 at 9:41 AM Karthik Nayak <karthik.188@gmail.com> wrote:

Show 10 quoted lines

> +Options for `drop`
> +~~~~~~~~~~~~~~~~~~~~
> +
> +--all::
> +       Drop the reflogs of all references from all worktrees.
> +
> +--single-worktree::
> +       By default when `--all` is specified, reflogs from all working
> +       trees are dropped. This option limits the processing to reflogs
> +       from the current working tree only.

It seems to me that "--current-worktree" would have been clearer than "--single-worktree", but I understand that it would have been confusing to have a different name for basically the same option in `git reflog expire` and `git reflog drop`.

Show 15 quoted lines

> +       argc = parse_options(argc, argv, prefix, options, reflog_drop_usage, 0);
> +
> +       if (argc && do_all)
> +               usage(_("references specified along with --all"));
> +
> +       if (do_all) {
> +               struct worktree_reflogs collected = {
> +                       .reflogs = STRING_LIST_INIT_DUP,
> +               };
> +               struct string_list_item *item;
> +               struct worktree **worktrees, **p;
> +
> +               worktrees = get_worktrees();
> +               for (p = worktrees; *p; p++) {
> +                       if (single_worktree && !(*p)->is_current)

It looks like 'single_worktree' is only used here. This means that if a user forgets to add --all and only uses --single-worktree, nothing will happen and it seems to me that the command will exit with code 0. Even if `git reflog expire` already works like that, I think this is a bit unfortunate.

Otherwise this patch series looks very well done to me.

Thanks!