Closed Bug 1026811 Opened 6 years ago Closed 6 years ago

Upgrade to CodeMirror 4.2

Categories

(DevTools :: Source Editor, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file, 1 obsolete file)

We need to make a patch change on top of tern.js for Bug 1026560, so we should bump to the latest version of CodeMirror before doing so. 

Here is a list of changes since 4.0 (taken from http://codemirror.net/doc/releases.html):

19-05-2014: Version 4.2:

    Fix problem where some modes were broken by the fact that empty tokens were forbidden.
    Several fixes to context menu handling.
    On undo, scroll change, not cursor, into view.
    Rewritten Jade mode.
    Various improvements to Shell (support for more syntax) and Python (better indentation) modes.
    New mode: Cypher.
    New theme: Neo.
    Support direct styling options (color, line style, width) in the rulers addon.
    Recognize per-editor configuration for the show-hint and foldcode addons.
    More intelligent scanning for existing close tags in closetag addon.
    In the Vim bindings: Fix bracket matching, support case conversion in visual mode, visual paste, append action.
    Full list of patches.

22-04-2014: Version 4.1:

    Slightly incompatible: The "cursorActivity" event now fires after all other events for the operation (and only for handlers that were actually registered at the time the activity happened).
    New command: insertSoftTab.
    New mode: Django.
    Improved modes: Verilog (rewritten), Jinja2, Haxe, PHP (string interpolation highlighted), JavaScript (indentation of trailing else, template strings), LiveScript (multi-line strings).
    Many small issues from the 3.x→4.x transition were found and fixed.
    Full list of patches.
Duplicate of this bug: 1005857
No longer blocks: 1026560
Attached patch codemirror-4.2.patch (obsolete) — Splinter Review
Rob, this bumps up the version to 4.2.  It also reorganizes the codemirror/ folder to match the structure of the lib a bit more (which will make future updates easier):

codemirror/javascript.js -> codemirror/mode/javascript.js
codemirror/xml.js -> codemirror/mode/xml.js
codemirror/css.js -> codemirror/mode/css.js
codemirror/htmlmixed.js -> codemirror/mode/htmlmixed.js
codemirror/clike.js -> codemirror/mode/clike.js
codemirror/active-line.js -> codemirror/selection/active-line.js
codemirror/trailingspace.js -> codemirror/edit/trailingspace.js
codemirror/matchbrackets.js -> codemirror/edit/matchbrackets.js
codemirror/closebrackets.js -> codemirror/edit/closebrackets.js
codemirror/comment.js -> codemirror/comment/comment.js
codemirror/show-hint.js -> codemirror/hint/show-hint.js
codemirror/tern.js -> codemirror/tern/tern.js

I've also updated the readme for the file listing (which was a bit out of date in a couple of places anyway).

Feel free to reassign the review, I didn't know who else to send it to.  Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=fa7d2115d7fb
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attachment #8442045 - Flags: review?(rcampbell)
Hrm, there is a test failing because there is no callback parameter to term.showType() any more.  Looking at the code, I'm wondering if this is an undocumented patch on top of the library: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/sourceeditor/codemirror/tern.js#224.  Just looking at a random commit from 2013 on CodeMirror I never see this "c" parameter to the function.

Once Bug 1026560 lands we can actually emit the show-information event inside of the typeTip callback instead of having a patched version of the tern.js file
I've filed https://github.com/marijnh/CodeMirror/pull/2644 on the CodeMirror project to see if we can port this customization back into the project to avoid patching in the future.
Same patch, but with PR from Comment 4 applied: https://tbpl.mozilla.org/?tree=Try&rev=5dca837e59a0
Attachment #8442045 - Attachment is obsolete: true
Attachment #8442045 - Flags: review?(rcampbell)
Attachment #8442810 - Flags: review?(rcampbell)
Comment on attachment 8442810 [details] [diff] [review]
codemirror-4.2.patch

Review of attachment 8442810 [details] [diff] [review]:
-----------------------------------------------------------------

this looks fine to me. r+ with a successful try run.
Attachment #8442810 - Flags: review?(rcampbell) → review+
Green try run in Comment 5
Keywords: checkin-needed
The custom changes in tern.js have now been merged into CodeMirror so we won't need to port them over next time: https://github.com/marijnh/CodeMirror/pull/2644
https://hg.mozilla.org/integration/fx-team/rev/55b0aa2da501
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/55b0aa2da501
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.