Chromium Code Reviews Chromium Code Reviews
Issue
748863003:
gn format: penalty-based scheme for line breaking (Closed)
Descriptiongn format: penalty-based scheme for line breaking
Previously, determining where newlines were inserted for line breaking was
primarily top-down. Some parts of the code asked for the length of its
subexpressions, and then made a decision on how to split based on how wide
those were, but the subexpressions were evaluated without any context.
This breaks down in more complicated cases, in particular when the
subexpressions are more complicated than the parent one.
The new approach is to attempt various layouts and score them based on
a penalty scheme. Things like exceeding the column limit have a large
penalty, things like causing additional horizontal separation or adding
more lines than necessary have lesser penalties. There is also a bias
towards splitting higher in the parse tree which balances expressions
better. e.g.
args = rebase_path(sources, root_build_dir) + rebase_path(outputs,
root_build_dir)
vs.
args = rebase_path(sources, root_build_dir) +
rebase_path(outputs, root_build_dir)
are both correct and take the same number of lines, but the second is
nicer to read.
This will be doing a lot more work in deep expressions, which seems like
it could be a performance issue in pathological cases. In practice,
formatting all of Chrome's BUILD.gn's is still fast so it's OK for now.
The work is repetitive, and could be cached if necessary in the future.
There are also some other fixes to continuation of indents that were
related to this change. It also rolls in a version of
https://codereview.chromium.org/731923003/ (making distinguished items
multiline per style guide).
A sample run of reformatting all of Chrome after this is at:
https://codereview.chromium.org/704363002 .
(This was a bit of a battle:
Nope! https://codereview.chromium.org/742003002/
Nope! https://codereview.chromium.org/750373002/
Nope! https://codereview.chromium.org/750373002/ .
This one is less generic, but seems to work well.)
R=brettw@chromium.org
BUG=348474
Committed: https://crrev.com/fede808424d25f2767f58c01a4cd30f7f053a32d
Cr-Commit-Position: refs/heads/master@{#305881}
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : . #Patch Set 4 : . #Patch Set 5 : integrating penalties #Patch Set 6 : func call fixes #Patch Set 7 : higher linebreak penalty for deeper in tree, and for horizontal separation between lhs/rhs of binop #Patch Set 8 : nice function formatting #Patch Set 9 : wip on alignment #Patch Set 10 : wip, mucking with alignment #Patch Set 11 : . #Patch Set 12 : . #Patch Set 13 : . #Patch Set 14 : fix suffixes not being taken into account in measurement #Patch Set 15 : propagate penalties #Patch Set 16 : rebase #Patch Set 17 : . #
Total comments: 1
Patch Set 18 : x64 #Messages
Total messages: 10 (2 generated)
|
