Bug 130019 - Web Inspector: Consider updating to CodeMirror 4.0
Summary: Web Inspector: Consider updating to CodeMirror 4.0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-03-10 06:07 PDT by Timothy Hatcher
Modified: 2014-04-14 21:32 PDT (History)
6 users (show)

See Also:


Attachments
[PATCH] Proposed Fix: Update to CodeMirror 4.0. (887.47 KB, patch)
2014-04-14 17:32 PDT, Jonathan Wells
timothy: review-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix: Update to CodeMirror 4.0. (874.72 KB, patch)
2014-04-14 19:17 PDT, Jonathan Wells
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.