Attachment 342354 Details for Bug 186453 – Patch

[patch] Patch

bug-186453-20180608191838.patch (text/plain), 7.23 KB, created by Matt Baker on 2018-06-08 19:18:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2018-06-08 19:18:39 PDT

Size: 7.23 KB

patch

obsolete

>Subversion Revision: 232591
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 1845678c9b8e7e1e4ad75ef7cce27793835e1ef6..d501edad60ea542b472a8f2fc76cdb76052accb3 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-06-08  Matt Baker  <mattbaker@apple.com>
>+
>+        Web Inspector: REGRESSION (r195723): CodeMirrorEditor shouldn't force line endings to "\n"
>+        https://bugs.webkit.org/show_bug.cgi?id=186453
>+        <rdar://problem/39689180>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Esprima token locations should be identified by line and column number,
>+        instead of character offset, since the former is line-ending agnostic.
>+
>+        * UserInterface/Controllers/TypeTokenAnnotator.js:
>+        (WI.TypeTokenAnnotator.prototype._insertTypeToken):
>+        (WI.TypeTokenAnnotator.prototype._insertToken):
>+
>+        * UserInterface/Models/ScriptSyntaxTree.js:
>+        (WI.ScriptSyntaxTree):
>+        (WI.ScriptSyntaxTree.prototype._createInternalSyntaxTree):
>+        Request per-token location (line, column) from Esprima. Esprima
>+        locations are converted to the format CodeMirror expects before
>+        being added to the AST node.
>+
>+        * UserInterface/Views/CodeMirrorEditor.js:
>+        (WI.CodeMirrorEditor.create):
>+        (WI.CodeMirrorEditor):
>+        Do not specify a line separator.
>+
>+        * UserInterface/Views/TextEditor.js:
>+        (WI.TextEditor.set string.update):
>+        (WI.TextEditor.prototype.set string):
>+        Remove assertion, as it will no longer be true.
>+
> 2018-06-05  Matt Baker  <mattbaker@apple.com>
> 
>         Web Inspector: Tab picker is briefly visible when TabBar initially shown
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js b/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js
>index bf1cc019ae87b8b28dcc3e98c0e1d5bf301da7cf..d6b5e6b167a649d5969dc90ed381a11a3ab7981c 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js
>@@ -86,7 +86,7 @@ WI.TypeTokenAnnotator = class TypeTokenAnnotator extends WI.Annotator
>     {
>         if (node.type === WI.ScriptSyntaxTree.NodeType.Identifier) {
>             if (!node.attachments.__typeToken && node.attachments.types && node.attachments.types.valid)
>-                this._insertToken(node.range[0], node, false, WI.TypeTokenView.TitleType.Variable, node.name);
>+                this._insertToken(node, false, WI.TypeTokenView.TitleType.Variable, node.name);
> 
>             if (node.attachments.__typeToken)
>                 node.attachments.__typeToken.update(node.attachments.types);
>@@ -105,16 +105,16 @@ WI.TypeTokenAnnotator = class TypeTokenAnnotator extends WI.Annotator
>         var scriptSyntaxTree = this._script._scriptSyntaxTree;
>         if (!node.attachments.__typeToken && (scriptSyntaxTree.containsNonEmptyReturnStatement(node.body) || !functionReturnType.typeSet.isContainedIn(WI.TypeSet.TypeBit.Undefined))) {
>             var functionName = node.id ? node.id.name : null;
>-            this._insertToken(node.typeProfilingReturnDivot, node, true, WI.TypeTokenView.TitleType.ReturnStatement, functionName);
>+            this._insertToken(node, true, WI.TypeTokenView.TitleType.ReturnStatement, functionName);
>         }
> 
>         if (node.attachments.__typeToken)
>             node.attachments.__typeToken.update(node.attachments.returnTypes);
>     }
> 
>-    _insertToken(originalOffset, node, shouldTranslateOffsetToAfterParameterList, typeTokenTitleType, functionOrVariableName)
>+    _insertToken(node, shouldTranslateOffsetToAfterParameterList, typeTokenTitleType, functionOrVariableName)
>     {
>-        var tokenPosition = this.sourceCodeTextEditor.originalOffsetToCurrentPosition(originalOffset);
>+        var tokenPosition = node.tokenPosition;
>         var currentOffset = this.sourceCodeTextEditor.currentPositionToCurrentOffset(tokenPosition);
>         var sourceString = this.sourceCodeTextEditor.string;
> 
>diff --git a/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js b/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js
>index 5de64e8c81e223ba97941d1c27cafa8f97059df1..420ee77ad78fbb51b1fdb11540f7c0112c9680c8 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js
>@@ -33,7 +33,7 @@ WI.ScriptSyntaxTree = class ScriptSyntaxTree
> 
>         try {
>             let sourceType = this._script.sourceType === WI.Script.SourceType.Module ? "module" : "script";
>-            let esprimaSyntaxTree = esprima.parse(sourceText, {range: true, sourceType});
>+            let esprimaSyntaxTree = esprima.parse(sourceText, {loc: true, range: true, sourceType});
>             this._syntaxTree = this._createInternalSyntaxTree(esprimaSyntaxTree);
>             this._parsedSuccessfully = true;
>         } catch (error) {
>@@ -1058,6 +1058,11 @@ WI.ScriptSyntaxTree = class ScriptSyntaxTree
>             return null;
>         }
> 
>+        let location = node.loc.start;
>+        result.tokenPosition = {
>+            line: location.line - 1,
>+            ch: location.column
>+        };
>         result.range = node.range;
>         // This is an object for which you can add fields to an AST node without worrying about polluting the syntax-related fields of the node.
>         result.attachments = {};
>diff --git a/Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js b/Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js
>index a0338c2cb4d87f030ee04fba5151b5ac34ea2356..66e47eeb01d562c493c53d0a3679c49544a71dab 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js
>@@ -27,9 +27,6 @@ WI.CodeMirrorEditor = class CodeMirrorEditor
> {
>     static create(element, options)
>     {
>-        if (options.lineSeparator === undefined)
>-            options.lineSeparator = "\n";
>-
>         // CodeMirror's manual scrollbar positioning results in double scrollbars,
>         // nor does it handle braces and brackets well, so default to using LTR.
>         // Clients can override this if custom layout for RTL is available.
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TextEditor.js b/Source/WebInspectorUI/UserInterface/Views/TextEditor.js
>index 5f6e24f00dc9509aa86588f66555b8eb811a26e5..7da80933f0285fc41940fbc5ae6eabecd0c83935 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TextEditor.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TextEditor.js
>@@ -118,10 +118,9 @@ WI.TextEditor = class TextEditor extends WI.View
>             if (this._initialStringNotSet)
>                 this._codeMirror.removeLineClass(0, "wrap");
> 
>-            if (this._codeMirror.getValue() !== newString) {
>+            if (this._codeMirror.getValue() !== newString)
>                 this._codeMirror.setValue(newString);
>-                console.assert(this.string.length === newString.length, "A lot of our code depends on precise text offsets, so the string should remain the same.");
>-            } else {
>+            else {
>                 // Ensure we at display content even if the value did not change. This often happens when auto formatting.
>                 this.layout();
>             }

Actions: View | Formatted Diff | Diff

Attachments on bug 186453: 342354 | 342358 | 342492 | 342597 | 342645 | 344956