Subject: Re: [PATCH 3/3] worktree list: keep the list sorted

From: Junio C Hamano <[email-masked]>
Date: Wed, 23 Nov 2016 09:16:46 -0800

Nguyễn Thái Ngọc Duy  <[email-masked]> writes:

> +		for (i = nr = 0; worktrees[i]; i++)
> +			nr++;
> +
> +		/*
> +		 * don't sort the first item (main worktree), which will
> +		 * always be the first
> +		 */
> +		QSORT(worktrees + 1, nr - 1, compare_worktree);
> +

This is somewhat curious.

	for (i = 0; worktrees[i]; i++)
		; /* just counting */
	QSORT(worktrees + 1, i - 1, compare_worktree);

would have been a lot more idiomatic (you do not use nr after sorting).

More importantly, perhaps get_worktrees() should learn to take an
optional pointer to int that returns how many items are in the list?

Alternatively, other existing callers of the function do not care
about the order, so it may not be such a good idea to always sort
the result, but it may be a good idea to teach it to take a boolean
that signals that the list should be sorted in a "natural order",
which is how "worktree list" would show them to the user?

This should be easily protectable with a new test.  Please do.