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

Diego Novillo dnovillo at google.com
Wed Sep 17 08:17:13 PDT 2014


On Wed, Sep 17, 2014 at 11:12 AM, Justin Bogner <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!


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


More information about the llvm-commits mailing list