Re: [PATCH 1/3] reflog: libify delete reflog function and helpers
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 18, 2022, 19:48 UTC
- Message-ID
- <220218.86bkz4gdlt.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <Yg/1205sTdqE2mC5@nand.local>
On Fri, Feb 18 2022, Taylor Blau wrote:
Show 40 quoted lines
> On Fri, Feb 18, 2022 at 08:10:07PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> > +int reflog_delete(const char *rev, int flags, int verbose)
>> > +{
>> > + struct cmd_reflog_expire_cb cmd = { 0 };
>> > + int status = 0;
>> > + reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
>> > + const char *spec = strstr(rev, "@{");
>> > + char *ep, *ref;
>> > + int recno;
>> > + struct expire_reflog_policy_cb cb = {
>> > + .dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
>> > + };
>> > +
>> > + if (verbose)
>> > + should_prune_fn = should_expire_reflog_ent_verbose;
>> > +
>> > + if (!spec) {
>> > + status |= error(_("not a reflog: %s"), rev);
>> > + }
>> > +
>> > + if (!dwim_log(rev, spec - rev, NULL, &ref)) {
>> > + status |= error(_("no reflog for '%s'"), rev);
>> > + }
>>
>> For these let's follow our usual style of not having braces for
>> single-line if's.
>>
>> Buuuut in this case doing so will make the diff move detection less
>> useful for 1..2.
>>
>> So probably best to leave it, or do some post-cleanup at the end maybe.
>
> Hmm. I don't think the diff detection mechanism would have an
> opportunity to kick in here, since the code is added in one patch and
> then removed in another. I think I may be missing what you're trying to
> say here ;).
>
> In any case, I don't think it's a huge deal if we can't accurately
> colorize this with `--color-moved`, so I'd probably just as soon clean
> up the style nits in this patch.Yes, this should really be read in combination with my comment on 2/3 to squash that commit into 1/3 to take full advantage of that, sorry.