Bug 16397 – Patch to pretty-print JavaScript

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Brent Fulgham]

I was playing with Drosera on Amazon.com and got tired of being unable to test any features due to the one-line javascript implementations they use.

I put together the attached patch to give rudimentary indention for javascript files.  It works reasonably well, though it could probably be improved considerably by a knowledgeable JavaScript hacker.

[Brent Fulgham]

Created attachment 17839 [details]
Patch to enable javascript indent in Drosera.

[Alexey Proskuryakov]

Yay - this is a huge problem when debugging sites!

Is this patch intended for review? Please mark it as such via Edit link if it is.

[Brent Fulgham]

Comment on attachment 17839 [details]
Patch to enable javascript indent in Drosera.

>Index: ChangeLog
>===================================================================
>--- ChangeLog	(revision 28609)
>+++ ChangeLog	(working copy)
>@@ -1,3 +1,9 @@
>[phone]  Brent Fulgham  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * Drosera/debugger.js:
>+
> 2007-12-10  Brady Eidson  <[e-mail]>
> 
>         Rubberstamped by Sam Weinig
>Index: Drosera/debugger.js
>===================================================================
>--- Drosera/debugger.js	(revision 28609)
>+++ Drosera/debugger.js	(working copy)
>@@ -713,6 +713,61 @@ function syntaxHighlight(code, file)
>         return false;
>     }
> 
>+    function nextNonWSisClosingBrace(currLoc) {
>+        // scan forward.  If the first thing we hit is a closing brace, must decrement indent.
>+        var sz  = ((code.length - currLoc) > 80) ? 80 : code.length - currLoc - 1;
>+        var sub = code.substr(currLoc, sz);
>+        var pat = /\S/;
>+        var pos = sub.search(pat);
>+        if (pos >= 0 && code.charAt(pos + currLoc) == "}") {
>+            return true;
>+        }
>+
>+        return false;
>+    }
>+
>+    var findEOL = /\s*\n/;
>+    var indentLevel = 0;
>+    var indentSize = 4;        // 4 spaces per indent.  Should be configurable!
>+
>+    function indentLine(currLoc, indentLevel) {
>+        var c = code.charAt(currLoc);
>+        var indentCount = 0;
>+
>+        if (nextNonWSisClosingBrace(currLoc)) {
>+            if (indentLevel > 0)
>+                --indentLevel;
>+        }
>+
>+        if (0 == indentLevel)
>+            return currLoc;
>+
>+        while (c == " " || c == "\t" || c == "\n") {
>+            if (indentCount < indentLevel * indentSize) {
>+                if (c == "\t") {
>+                    for (var j = 0; j < indentSize; ++j)
>+                        echoChar(" ");
>+                } else if (c == "\n") {
>+                    echoChar(" ");
>+                } else
>+                    echoChar(c);
>+            }
>+            currLoc++;
>+            c = code.charAt(currLoc);
>+            if (c == "\t")
>+                indentCount += indentSize;
>+            else
>+                indentCount++;
>+        }
>+
>+        while (indentCount < indentLevel * indentSize) { 
>+            echoChar(" ");
>+            indentCount++;
>+        }
>+
>+        return currLoc;
>+    }
>+
>     var result = "";
>     var cPrev = "";
>     var c = "";
>@@ -751,6 +806,8 @@ function syntaxHighlight(code, file)
>             }
>             result += "</span>";
>             echoChar(c);
>+            
>+            i = indentLine(i+1, indentLevel) - 1;
>             continue;
>         } else if (c == "\"" || c == "'") {
>             var instringtype = c;
>@@ -865,6 +922,28 @@ function syntaxHighlight(code, file)
> 
>                 continue;
>             }
>+        } else if (c == "{") {
>+            indentLevel++;
>+            echoChar(c);
>+            echoChar("\n");
>+            i = indentLine(i+1, indentLevel) - 1;
>+            continue;
>+        } else if (c == "}") {
>+
>+            if (cPrev != " " && cPrev != "\t" && cPrev != "\n" && cPrev != ";") {
>+                echoChar("\n");
>+                i = indentLine(i, indentLevel);
>+            }
>+
>+            echoChar(c);
>+            if (indentLevel > 0)
>+                indentLevel--;
>+            continue;
>+       } else if (c == ";") {
>+            echoChar(c);
>+            echoChar("\n");
>+            i = indentLine(i+1, indentLevel) - 1;
>+            continue;
>         }
> 
>         echoChar(c);

[Brent Fulgham]

Comment on attachment 17839 [details]
Patch to enable javascript indent in Drosera.

The patch unfortunately contains some changes related to another patch I submitted (Bug 16314).  Basically the only diff needed is the debugger.js file.

[Brent Fulgham]

Created attachment 17855 [details]
Patch to provide rudimentary indentation for JavaScript.

Removed extraneous changes from unrelated patch.

[Matt Lilek]

In the future, you should include the title of the bug report and a link to it in the ChangeLog.

[Timothy Hatcher]

Comment on attachment 17855 [details]
Patch to provide rudimentary indentation for JavaScript.

This patch seems good and is a good step. However, this will break breakpoints. The line numbers will not match and you will get random breaks that don't match the original code.

To really fix this we should remember what the original line was and make sure adding a breakpoint on the "virtual line" really adds a breakpoint for the original line.

Pretty printing should also be optional, and off by default. We should have a toolbar button, or something to toggle between pretty print and the original.

Also as mentioned on IRC by bdash, syntax highlighting and pretty printing should be done in JavaScriptCore. But that can be done later. It would help speed Drosera up too.

r- until breakpoints work and this is optional.

[Timothy Hatcher]

This should be added to the Web Inspector in some fashion now that Drosera doesn't exist and the debugger is in the Inspector.

[Keishi Hattori]

*** Bug 31118 has been marked as a duplicate of this bug. ***