Re: [PATCH v3 2/7] bisect: read bisect paths with strbuf_getline()

> On Sun, Feb 28, 2016 at 12:13 AM, Moritz Neeb <lists@moritzneeb.de> wrote:
>> The file BISECT_NAMES is written by "git rev-parse --sq-quote" via
>> sq_quote_argv() when starting a bisection. It can contain pathspecs
>> to narrow down the search. When reading it back, it should be expected that
>> sq_dequote_to_argv_array() is able to parse this file. In fact, the
>> previous commit ensures this.
>>
>> As the content is of type "text", that means there is no logic expecting
>> CR, strbuf_getline_lf() will be replaced by strbuf_getline().
>>
>> Apart from whitespace added and removed in quote.c, no more whitespaces
>> are expexted. While it is technically possible, we have never advertised
> 
> s/expexted/expected/
> 
>> this file to be editable by user, or encouraged them to do so, thus
>> the call to strbuf_trim() turns obsolete in various ways.
> 
> Not sure what "various ways" you mean. Perhaps say instead that (as a
> consequence of "not advertised or encouraged") you're tightening the
> parsing of this file by removing strbuf_trim().

> 
>> For the case that this file is modified nonetheless, in an invalid way
>> such that dequoting fails, the error message is broadened to both cases:
>> bad quoting and unexpected whitespace.
>>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Moritz Neeb <lists@moritzneeb.de>
>> ---
>> diff --git a/bisect.c b/bisect.c
>> @@ -440,10 +440,9 @@ static void read_bisect_paths(struct argv_array *array)
>>         if (!fp)
>>                 die_errno("Could not open file '%s'", filename);
>>
>> -       while (strbuf_getline_lf(&str, fp) != EOF) {
>> -               strbuf_trim(&str);
>> +       while (strbuf_getline(&str, fp) != EOF) {
>>                 if (sq_dequote_to_argv_array(str.buf, array))
>> -                       die("Badly quoted content in file '%s': %s",
>> +                       die("Badly quoted content or unexpected whitespace in file '%s': %s",
>>                             filename, str.buf);
>>         }
>>
>> --
>> 2.4.3