Bug 130019

Summary: Web Inspector: Consider updating to CodeMirror 4.0
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, jonowells, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix: Update to CodeMirror 4.0.
timothy: review-
[PATCH] Proposed Fix: Update to CodeMirror 4.0. none

Description Timothy Hatcher 2014-03-10 06:07:24 PDT
CodeMirror 4.0 is about to be released. We should give it a try by testing and maybe updating to the release candidate.

The major new features in 4.0 are: 
 - Multiple selections (ctrl-click, alt-drag, and API functions to 
work with them) 
 - Selection undo/redo (ctrl-u to undo selection and alt-u to redo 
selection in the default keymap) 
 - A set of Sublime Text keybindings (see [4]) 
 - All modules are wrapped in AMD and CommonJS module loader shims, so 
that they play well with module loaders. When neither is present, the 
old style (global CodeMirror variable) is used. 
 - A new character measuring subsystem that is faster (especially on 
long lines) and more robust (no more problematic corner cases in 
wrapped text). 

The last point is the most interesting to me and should fix some long standing issues.
Comment 2 Radar WebKit Bug Importer 2014-04-08 16:43:02 PDT
<rdar://problem/16559681>
Comment 3 Timothy Hatcher 2014-04-08 16:44:01 PDT
CodeMirror 4 has been released. We should do this.
Comment 4 Jonathan Wells 2014-04-14 17:32:52 PDT
Created attachment 229328 [details]
[PATCH] Proposed Fix: Update to CodeMirror 4.0.
Comment 5 Timothy Hatcher 2014-04-14 18:12:49 PDT
Comment on attachment 229328 [details]
[PATCH] Proposed Fix: Update to CodeMirror 4.0.

View in context: https://bugs.webkit.org/attachment.cgi?id=229328&action=review

Looking good. Some minor issues. Also need to rebase so the patch can apply on TOT to land.

> Source/WebInspectorUI/ChangeLog:219
>          * Localizations/en.lproj/localizedStrings.js:
>          * UserInterface/Models/DOMNode.js:
>          * UserInterface/Views/DOMNodeDetailsSidebarPanel.js:
> -        * UserInterface/Views/Main.css:
> +        * UserInterface/Views/Main.css:
>  
>  2014-03-28  Joseph Pecoraro  <pecoraro@apple.com>
>  

You should revert the edits to the old parts of the ChangeLog.

> Source/WebInspectorUI/Scripts/update-codemirror-resources.rb:-46
> -  mode/less/less.js

Need to remove less.js from Main.html. Not removing it from there will break the Production build. You should also test to make sure combining the resources like Production builds do still works. Especially jsmin.py still works on the new codemirror.js. Define COMBINE_INSPECTOR_RESOURCES=YES while building to trigger this. Or hack copy-user-interface-resources.pl to always do it.

> Source/WebInspectorUI/UserInterface/Views/ResourceDetailsSidebarPanel.js:383
> -
> +

This whole file can be reverted.
Comment 6 Jonathan Wells 2014-04-14 19:17:04 PDT
Created attachment 229337 [details]
[PATCH] Proposed Fix: Update to CodeMirror 4.0.
Comment 7 Jonathan Wells 2014-04-14 19:19:13 PDT
The production build seems to combine the assets correctly with the new CodeMirror.
Comment 8 WebKit Commit Bot 2014-04-14 21:32:13 PDT
Comment on attachment 229337 [details]
[PATCH] Proposed Fix: Update to CodeMirror 4.0.

Clearing flags on attachment: 229337

Committed r167294: <http://trac.webkit.org/changeset/167294>
Comment 9 WebKit Commit Bot 2014-04-14 21:32:16 PDT
All reviewed patches have been landed.  Closing bug.