Re: [PATCH 2/8] xdl_change_compact(): clarify code

From: Stefan Beller <sbeller@google.com>
Date: Aug 10, 2016, 16:58 UTC
Message-ID: <CAGZ79kaSnm-MoWf254UpzY4XS=AO57fChqOntBzGF3wW6TxUag@mail.gmail.com>
In-Reply-To: <48ca79e2-418b-3c73-e075-808d39a4da9a@alum.mit.edu>

On Wed, Aug 10, 2016 at 9:39 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 5 quoted lines

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

Oh :(

I think there was some discussion when we added the blank line counting whether we would want to have it boolean or counting. And we settled for counting as "future algorithms can make use of this additional information" IIRC.

Show 9 quoted lines

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

No objections from my side, sorry for this lengthy discussion about a comment,

Thanks, Stefan