See summary.
Created attachment 78644 [details] Patch
Comment on attachment 78644 [details] Patch At least one person said this was hard to understand so I'll try to make the code easier to read.
Comment on attachment 78644 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78644&action=review No, I was so sick and my brain stopped working when I told you that I couldn't understand it. Now I feel better and I can understand this patch :) I'm sure this code is good enough to land this as is. I put some nitpicks. > Tools/Scripts/webkitpy/style/checkers/cpp.py:817 > end_character = ')' How about creating the regexp string here? if start_character == '(': enclosing_characeter_regexp = r'[\{\}]' elif ... > Tools/Scripts/webkitpy/style/checkers/cpp.py:829 > + while True: I think you used while because we start in the middle of the first line, but I'd mildly prefer for-loop: current_column = position.column + 1 net_open = 1 for line_number in xrange(position.row, len(elided): line = elided[line_number][current_column:] while True: ... current_column = 0 return Position(len(elided), -1) because in this way we can clearly see the iterator of the outer loop.
Committed as http://trac.webkit.org/changeset/75628
(In reply to comment #3) > (From update of attachment 78644 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78644&action=review > > No, I was so sick and my brain stopped working when I told you that I couldn't understand it. Now I feel better and I can understand this patch :) I'm sure this code is good enough to land this as is. I put some nitpicks. > > > Tools/Scripts/webkitpy/style/checkers/cpp.py:817 > > end_character = ')' > > How about creating the regexp string here? > > if start_character == '(': > enclosing_characeter_regexp = r'[\{\}]' > elif > ... > Done. > > Tools/Scripts/webkitpy/style/checkers/cpp.py:829 > > + while True: > > I think you used while because we start in the middle of the first line, but I'd mildly prefer for-loop: > > current_column = position.column + 1 > net_open = 1 > for line_number in xrange(position.row, len(elided): > line = elided[line_number][current_column:] > while True: > ... > current_column = 0 > return Position(len(elided), -1) > > because in this way we can clearly see the iterator of the outer loop. I really like this. I did it (in a slightly modified way but basically the same idea).