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

From: Moritz Neeb <[email-masked]>
Date: Mon, 22 Feb 2016 01:07:34 +0100

On 02/15/2016 06:05 AM, Junio C Hamano wrote:
> 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.

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