[PATCH] LineIterator: Provide a variant that keeps blank lines

[Justin Bogner]

On Wednesday, September 17, 2014, Diego Novillo <dnovillo at google.com> wrote:

>
>
> On Wed, Sep 17, 2014 at 11:12 AM, Justin Bogner <mail at justinbogner.com
> <javascript:_e(%7B%7D,'cvml','mail at justinbogner.com');>> wrote:
>
>>
>> Also, skipping blank lines in your patch seems to have a counter
>>> intuitive semantics. If I'm skipping blank lines, it means I don't want to
>>> be returned a blank line, but I still expect the line number to be
>>> increased. In this patch, that does not seem to happen. In fact, the test
>>> here seems wrong:
>>>
>>
>> I think you've misread something here - the line number is increased when
>> we skip blank lines. The behaviour when SkipBlanks is true is unchanged
>> from without this patch.
>>
>
> Ah, yes, quite right.
>
>
> -  EXPECT_EQ("line 4", *I);
>>> -  EXPECT_EQ(4, I.line_number());
>>> +  EXPECT_EQ("line 5", *I);
>>> +  EXPECT_EQ(5, I.line_number());
>>> +  ++I;
>>>
>>> The string "line 5" is really at line 6, so I.line_number() should
>>> return 6, not 5.
>>>
>>
>> No, the string "Line 5" is really line 5. Note that an extra newline was
>> added to the test, so line 4 is now blank.
>>
>
> Yes, that's what confused me.
>
> Patch LGTM. Thanks!
>

Sorry about the confusion there, and thanks for the review! Committed in
r217960.


>
> Diego.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140917/b4d444ef/attachment.html>