Re: [PATCH 1/3] reflog: libify delete reflog function and helpers

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Feb 18 2022, Taylor Blau wrote:

> 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.