Re: [PATCH v2 5/7] ref-filter: move ref_sorting flags to a bitfield

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 7, 2021, 23:24 UTC
Message-ID: <xmqqpn2gjpoa.fsf@gitster.c.googlers.com>
In-Reply-To: <20210107095153.4753-6-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 5 quoted lines

> Perhaps there's a more idiomatic way of doing the "for each in list
> amend mask" pattern than this "mask/on" variable combo. This function
> doesn't allow us to e.g. do any arbitrary changes to the bitfield for
> multiple flags, but I think in this case that's fine. The common case
> is that we're calling this with a list of one.

An obvious alternative would be to pass two masks, one for setting and the other for clearing, instead of passing a mask and a bool that says if the mask is for setting or clearing.

The helper that follows such a design would be:

	void ref_sorting_tweak_flags(struct ref_sorting *sorting,
				     unsigned set, unsigned clear)
	{
		while (sorting) {
			sorting->sort_flags |= set;
			sorting->sort_flags &= ~clear;
			sorting = sorting->next;
		}
	}

and the caller in the endgame would become

	...
	} else if (list) {
		unsigned set = REF_SORTING_DETACHED_HEAD_FIRST;
		unsigned clear = 0;

                *(icase ? &set : &clear) |= REF_SORTING_ICASE;
		ref_sorting_tweak_flags(sorting, set, clear);

which may be more lines but probably copes better when adding new bits.

Thanks.