Re: [PATCH 1/5] bisect: read bisect paths with strbuf_getline()

From: Moritz Neeb <lists@moritzneeb.de>
Date: Feb 22, 2016, 00:07 UTC
Message-ID: <56CA5146.7050008@moritzneeb.de>
In-Reply-To: <xmqqtwlar2bd.fsf@gitster.mtv.corp.google.com>

On 02/15/2016 06:05 AM, Junio C Hamano wrote:

Show 7 quoted lines

> As to bisect pathspec, while I do not think it is sensible to change
> it in the middle of bisection, I do not think it would cause the
> bisect command to produce an incorrect result if you did so.  You
> would be changing the definition of what is the "middle point" of
> the history on the fly by changing the pathspec, hence you may end
> up making the bisection suboptimal, but it shouldn't affect the
> correctness of the bisection.

Yep I also think it would not affect the correctness. To reformulate your explanation: changing the pathspec does not change the good and bad "frontiers". And those should be still correct under the assumption you're still looking for the same bug/regression.

As to the question if it is sensible, I also don't really think it could be helpful. The only way I would expect it to be useful, it so "refine" the search by adding specific files to the filepath. But then it's possible to end up in a dead-end if the commits left are not touching the specified files with:

	No testable commit found.
	Maybe you started with bad path parameters?

I have also seen other messed up messages, like "-1 commits left" while playing around.

> So I tend to agree with you that it is a good idea to be prepared to
> parse what the end user may have futzed with the editor, as long as
> the user did not break the syntax of the quoted string.

Now I am not convinced anymore, because I think if it potentially leads to troubles, it should not be made easier to get oneself into them.

Show 5 quoted lines

> 	Side note: of course, you can instead say "while it is
> 	technically possible, we have never advertised this file to
> 	be editable by user, or encouraged them to do so" and
> 	tighten the parsing to assume only what we write out,
> 	erroring out when we see any attempt to edit it.

I will include something like that in the patch v2 that is to follow soon.