This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111540/

On July 16th, 2013, 6:43 p.m. EDT, Milian Wolff wrote:

part/syntax/katehighlight.cpp (Diff revision 1)
void KateHighlighting::doHighlight ( const Kate::TextLineData *_prevLine,
399	// without eating anything... this would be an infinite loop!	408	// without eating anything... this would be an infinite loop!

Here and below, I find it personally very offending reading your shouting comments. Please follow the code of conduct, see also the recent discussion happening in the Linux Kernel.






hehe j/k :] Still, why! do! you! feel! so! shouty! :D

I think you picked a bad example ;-). From just the line you commented on, I would have said that a '!' looked reasonable in this case, and felt inspired to come toss my 2¢ in. Then I looked at the rest of the diff, and have to agree, the others are... more unexpected :-).

(p.s. Are you intending to open issues? If not - and considering you also said 'ship it!' I would guesss 'not' is the case - you may want to uncheck that option in your user preferences...)

- Matthew

On July 16th, 2013, 5:25 p.m. EDT, Dominik Haumann wrote:

Review request for Kate, Christoph Cullmann and Milian Wolff.

By Dominik Haumann.

Updated July 16, 2013, 5:25 p.m.

Description

Currently, when using lookAhead="true", the beginRegion and endRegion attributes are not processed.
Therefore, a rule like the following does not work:
  <StringDetect String="x" lookAhead="true" endRegion="a" beginRegion="a" />

This patch fixes this. Therefore, using endRegion and beginRegion along with lookAhead will
- not process any characters as before, but
- folding regions are still processed.

This effectively means, that TextLineData::addAttribute (const Attribute &attribute) is called as follows:
one time for endRegion:
Attribute(offset, length=0, someAttribute (does not matter, since length=0), someFoldingId)
one time for startRegion:
Attribute(offset, length=0, someAttribute (does not matter, since length=0), someFoldingId)

The endRegion is pushed first, i.e. before the startRegion.

The changes are as follows:
1. The folding stuff is moved out of the if (!lookAhead) {...} scope, and therefore always executed
2. Therewith, the if (!lookAhead) {...} scope is split into two scopes. The first one is moved a bit up.

Testing

Minimal test, which works for me. No thorough testing, since we are missing a test suite for this anyways.

Bugs: 322076

Diffs

part/syntax/katehighlight.cpp (1a34395)

View Diff