New issue
Advanced search Search tips

Issue 790021 link

Starred by 1 user
Status: Fixed
Owner:
scottmg@chromium.org
Closed: Dec 12
Cc:
rsesek@chromium.org
mark@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Bad wrapping in gn format for one line configs modification

Project Member Reported by scottmg@chromium.org, Nov 30 2017 
From https://chromium-review.googlesource.com/c/crashpad/crashpad/+/797414/4/third_party/gtest/BUILD.gn#62

Example:

if (true) {
  configs -= [ "//third_party/mini_chromium/mini_chromium/build:Wexit_time_destructors" ]
}

This doesn't wrap, and so exceeds 80 col. This should instead probably be

if (true) {
  configs -= [
      "//third_party/mini_chromium/mini_chromium/build:Wexit_time_destructors" ]
}

or 


if (true) {
  configs -= [
      "//third_party/mini_chromium/mini_chromium/build:Wexit_time_destructors"
  ]
}
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f86d891eda0a8a4207a7c65fd353702ff15c26c

commit 7f86d891eda0a8a4207a7c65fd353702ff15c26c
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Nov 30 22:45:30 2017

gn: Check if formatting a single line list to multiple reduces penalty

Previously, when formatting binary operators, gn format would only try
to break around the operators, but the determination of whether a list
was single or multiline was only determined by the number of elements
in the list (and in some cases, the LHS of the assignment).

Instead now, check if spliting a single line list into multiline would
avoid going past 80 columns.

A test run of running this new behaviour on all of chromium/src is at
https://chromium-review.googlesource.com/c/chromium/src/+/801660/, and
it seems like a universal improvement, fixing a number of misformattings.

Bug:  790021 
Change-Id: I06ab6f06388dc9429a75407864769484d3fcea41
Reviewed-on: https://chromium-review.googlesource.com/801658
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520726}
[modify] https://crrev.com/7f86d891eda0a8a4207a7c65fd353702ff15c26c/tools/gn/command_format.cc
[modify] https://crrev.com/7f86d891eda0a8a4207a7c65fd353702ff15c26c/tools/gn/command_format_unittest.cc
[add] https://crrev.com/7f86d891eda0a8a4207a7c65fd353702ff15c26c/tools/gn/format_test_data/069.gn
[add] https://crrev.com/7f86d891eda0a8a4207a7c65fd353702ff15c26c/tools/gn/format_test_data/069.golden

Comment 2 by scottmg@chromium.org, Dec 12

Status: Fixed (was: Assigned)
►

Sign in to add a comment