Re: [PATCH 03/19] diff.c: drop 'nofirst' from emit_line_0

From: Jonathan Tan <jonathantanmy@google.com>
Date: May 16, 2017, 16:05 UTC
Message-ID: <CAGf8dgJY0=hr9_M3LijZnEhv7rEad8XHAjcpjG425JtKNjweHQ@mail.gmail.com>
In-Reply-To: <CAGZ79kZSixaG2CoO-WV47=-eE96yhtoCm82KFbZ=-=8c-VPXcA@mail.gmail.com>

On Mon, May 15, 2017 at 11:33 AM, Stefan Beller <sbeller@google.com> wrote:

Show 7 quoted lines

> On Mon, May 15, 2017 at 11:26 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
>> I also don't understand the meaning of this paragraph - if you mean that
>> this patch teaches other callers to hardcode the sign, I don't see any such
>> changes in the diff below.
>
> The last two hunks of the patch switch two callers that call with a sign
> that is hard to reason about.

The last two hunks don't hardcode any signs, as far as I can see. They do pass in a "first" character that may or may not be a sign, if that is what you mean.

In any case, can you reword that paragraph into an imperative statement (e.g. "teach X to...", "make X...")?

Show 12 quoted lines

>>> +                       char term[2];
>>> +                       term[0] = options->line_termination;
>>> +                       term[1] = '\0';
>>> +
>>> +                       emit_line(options, NULL, NULL,
>>> +                                 term, 1);
>>
>>
>> If options->line_termination is 0, this is actually a zero-length string
>> (not 1).
>
> So passing in !!options->line_termination should be fine?

Yes, that would work. I slightly prefer !!term[0].