clang-format in javascript file uses too many line breaks and white-space |
||||
Issue description
clang-format produced code that (choose all that apply):
- Riles my finely honed stylistic dander
- No sane human would ever choose
Here's the code after formatting:
+'use strict';
+promise_test(
+ () => {
+ return setBluetoothFakeAdapter('FailingGATTOperationsAdapter')
+ .then(
+ () => requestDeviceWithKeyDown(
+ {filters: [{services: [errorUUID(0xA0)]}]}))
+ .then(device => device.gatt.connect())
+ .then(gattServer => gattServer.getPrimaryService(errorUUID(0xA0)))
+ .then(
+ service => {
+ service.getCharacteristic(errorUUID(0xA1))
+ .then(characteristic => {
+ let tests = Promise.resolve();
+ gatt_errors_tests.forEach(testSpec => {
+ tests =
+ tests
+ .then(
+ () => characteristic.getDescriptor(
+ testSpec.uuid))
+ .then(
+ descriptor =>
+ assert_promise_rejects_with_message(
+ descriptor.CALLS([readValue()]),
+ testSpec.error,
+ testSpec.testName));
+ });
+ return tests;
+ })})},
+ 'FUNCTION_NAME fails. Should reject with appropriate error.');
What's up with:
- Too many line breaks for continuation
- Using 4 space indent (instead of 2) for continuation indent that is in a block.
Here's how it ought to look:
+'use strict';
+promise_test(
+ () => {
+ return setBluetoothFakeAdapter('FailingGATTOperationsAdapter')
+ .then(() => requestDeviceWithKeyDown(
+ {filters: [{services: [errorUUID(0xA0)]}]}))
+ .then(device => device.gatt.connect())
+ .then(gattServer => gattServer.getPrimaryService(errorUUID(0xA0)))
+ .then(service => {
+ service.getCharacteristic(errorUUID(0xA1))
+ .then(characteristic => {
+ let tests = Promise.resolve();
+ gatt_errors_tests.forEach(testSpec => {
+ tests = tests
+ .then(() => characteristic.getDescriptor(
+ testSpec.uuid))
+ .then(descriptor =>
+ assert_promise_rejects_with_message(
+ descriptor.CALLS([readValue()]),
+ testSpec.error,
+ testSpec.testName));
+ });
+ return tests;
+ })})},
+ 'FUNCTION_NAME fails. Should reject with appropriate error.');
Code review link for full files/context:
https://codereview.chromium.org/2637343002/diff/60001/third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/io-op-fails.js#newcode26
Style references:
Chromium style refers to Blink style, which for layout tests refers to google style.
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/writing_layout_tests.md#Coding-Style
https://google.github.io/styleguide/jsguide.html#formatting-block-indentation
https://google.github.io/styleguide/jsguide.html#formatting-line-wrapping
,
Jan 25 2017
scheib@: I'm game to push for any change to the default Chromium JS style that results in objectively better or subjectively prettier code. I would just ask that the subjective stuff be agreed upon. maybe take a look at the existing options to see if there's anything that might be more useful: https://clang.llvm.org/docs/ClangFormatStyleOptions.html also, fwiw: all the continuations look correct (4\s) and thing like conditions or new scopes look correct at 2\s. this code just does a ton of continuations. additionally, Google's newer ES6 style guide changes how wrapping works with dots: https://google.github.io/styleguide/jsguide.html#formatting-where-to-break
,
Feb 8 2017
Re: my request for no line breaks after e.g. ".then(" I see that new style argues that there should be a line break. So I'll go with style.
Re: indents. I see blocks that are indented by 4 instead of 2. See line examples:
04: + return setBluetoothFakeAdapter
and
12: + service.getCharacteristic(errorUUID(0xA1))
This is different than on e.g.:
15: + gatt_errors_tests.forEach(testSpec => {
I haven't found in configuration doc what might be causing that. I can't see easily what makes those types of blocks different in the example code.
,
Oct 26 2017
,
Nov 27
,
Jan 11
Issue has a component, but no priority. Updating to have default priority (Pri-2) |
||||
►
Sign in to add a comment |
||||
Comment 1 by thakis@chromium.org
, Jan 23 2017