On 08/04/2016 01:50 AM, Stefan Beller wrote:
> On Wed, Aug 3, 2016 at 4:14 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
>> On 08/04/2016 12:11 AM, Stefan Beller wrote:
>>> On Wed, Aug 3, 2016 at 3:00 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
>>>> [...]
>>>> +
>>>> + /*
>>>> + * Are there any blank lines that could appear as the last
>>>> + * line of this group?
>>>> + */
>>>
>>> IIRC this comment is not quite correct as this 'only' counts the number of
>>> blank lines within the forward shifting section, i.e. in the movable space.
>>>
>>> Later we use it as a boolean indicator (whether or not it is equal to 0)
>>> to see if we can do better.
>>> [...]
Thanks for your comments, Stefan.
I realized that the main thing that took me a while to grok when I was
reading this code was that blank_lines was really only used as a boolean
value, even though it was updated with "+=". That's the main information
that I'd like to convey to the reader.
So I decided to change the comment to emphasize this fact (and change it
from a question to a statement), and also changed the place that
blank_lines is updated to treat it more like a boolean. The latter
change also has the advantage of not calling is_blank_line()
unnecessarily when blank_lines is already true.
If you have no objections, that is what I will put in v2 of this patch
series:
> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> index de15de2..fde0433 100644
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -460,6 +460,12 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
>
> do {
> groupsize = i - start;
> +
> + /*
> + * Boolean value that records whether there are any blank
> + * lines that could be made to be the last line of this
> + * group.
> + */
> blank_lines = 0;
>
> /*
> @@ -511,7 +517,8 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
> * the current change group.
> */
> while (i < nrec && recs_match(recs, start, i, flags)) {
> - blank_lines += is_blank_line(recs, i, flags);
> + if (!blank_lines)
> + blank_lines = is_blank_line(recs, i, flags);
>
> rchg[start++] = 0;
> rchg[i++] = 1;
Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html