New issue
Advanced search Search tips

Issue 708493 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

DevTools: Double parentheses in if-then-else cause permanent indentation level increase on pretty print

Reported by yellow...@gmail.com, Apr 5 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.41 Safari/537.36

Steps to reproduce the problem:
1. Save html.html and js.js into a directory.
2. Open html.html.
3. Open js.js in Sources tab.
4. Press "Pretty print".
Note how a second parenthesis is moved to a new line with indentation level increase, causing the subsequent code to form a "staircase" from accumulated additional indentation.

What is the expected behavior?
For
function test() {
	if((c))a;else b;
	if((c))a;else b;
	if((c))a;else b;
}
the expected output is
function test() {
    if ((c))
        a;
    else
        b;
    if ((c))
        a;
    else
        b;
    if ((c))
        a;
    else
        b;
}
(or equivalent)

What went wrong?
Indentation level is not restored after if-then-else branch ends, therefore you get (png1.png)
function test() {
    if ((c)
        )
            a;
        else
            b;
        if ((c)
            )
                a;
            else
                b;
            if ((c)
                )
                    a;
                else
                    b;
            }

Did this work before? No 

Chrome version: 58.0.3029.41  Channel: beta
OS Version: 10.0
Flash Version: 

This seems to exist for a while now, but I've only recently noticed the exact pattern (double parentheses in a condition with an else-branch) that causes this.
 
html.html
52 bytes View Download
js.js
77 bytes View Download
png1.png
7.0 KB View Download
Owner: lushnikov@chromium.org
Status: Assigned (was: Unconfirmed)
lushnikov@ can you please look into this?
Owner: l...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2017

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

commit 079dd5c40c3a59e588777c794a533f40755f8402
Author: luoe <luoe@chromium.org>
Date: Thu Apr 20 02:59:19 2017

DevTools: make JSFormatter aware of parenthesized expressions

JavascriptFormatter calls _beforeVisit() to collect tokens before visiting AST
nodes. However, the Acorn-provided AST did not generate enough info to
distinguish when the last ')' appeared, and our formatter would incorrectly add
a new line and indent whenever it saw any ')'.

This resulted in bad cases with expressions within more than one set of
parentheses (`if ((c))a;else b`).

This CL turns on an Acorn flag 'preserveParens' in order to distinguish and
insert a new line only for ')'s outside of a parenthesized expression.

Other examples:
with((c))a;
for (i=0; i< 5; (i++)){a;}
while ((true))x;

BUG= 708493 

Review-Url: https://codereview.chromium.org/2823233004
Cr-Commit-Position: refs/heads/master@{#465874}

[add] https://crrev.com/079dd5c40c3a59e588777c794a533f40755f8402/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-javascript-9-expected.txt
[add] https://crrev.com/079dd5c40c3a59e588777c794a533f40755f8402/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-javascript-9.html
[modify] https://crrev.com/079dd5c40c3a59e588777c794a533f40755f8402/third_party/WebKit/Source/devtools/front_end/formatter_worker/ESTreeWalker.js
[modify] https://crrev.com/079dd5c40c3a59e588777c794a533f40755f8402/third_party/WebKit/Source/devtools/front_end/formatter_worker/JavaScriptFormatter.js

Comment 4 by l...@chromium.org, Apr 20 2017

Status: Fixed (was: Assigned)
Summary: DevTools: Double parentheses in if-then-else cause permanent indentation level increase on pretty print (was: Double parentheses in if-then-else cause permanent indentation level increase on pretty print)

Sign in to add a comment