Bug 141011

Summary: Web Inspector: Make BasicBlockAnnotator lessen the saturation of syntax highlighting instead of graying out unexecuted code
Product: WebKit Reporter: Saam Barati <saam>
Component: Web InspectorAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch timothy: review+

Saam Barati
Reported 2015-01-28 14:45:52 PST
This provides a nicer user experience.
Attachments
patch (11.90 KB, patch)
2015-01-29 11:21 PST, Saam Barati
timothy: review+
Radar WebKit Bug Importer
Comment 1 2015-01-28 14:46:10 PST
Saam Barati
Comment 2 2015-01-29 11:21:19 PST
Timothy Hatcher
Comment 3 2015-01-29 12:32:29 PST
Comment on attachment 245635 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=245635&action=review > Source/WebInspectorUI/UserInterface/Controllers/BasicBlockAnnotator.js:-106 > - if (this._isTextRangeOnlyWhitespace(startPosition, endPosition) || this._isTextRangeOnlyClosingBrace(startPosition, endPosition)) > - return null; Should we keep the _isTextRangeOnlyClosingBrace check?
Saam Barati
Comment 4 2015-01-29 16:24:17 PST
(In reply to comment #3) > Comment on attachment 245635 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245635&action=review > > > Source/WebInspectorUI/UserInterface/Controllers/BasicBlockAnnotator.js:-106 > > - if (this._isTextRangeOnlyWhitespace(startPosition, endPosition) || this._isTextRangeOnlyClosingBrace(startPosition, endPosition)) > > - return null; > > Should we keep the _isTextRangeOnlyClosingBrace check? I don't think it's necessary. It shouldn't affect the UI in any way with the risk that it complicates the code and also uses more computation resources because that regexp will execute every time because if a range is white space only, we won't cache it.
Saam Barati
Comment 5 2015-01-29 16:25:04 PST
(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 245635 [details] > > patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=245635&action=review > > > > > Source/WebInspectorUI/UserInterface/Controllers/BasicBlockAnnotator.js:-106 > > > - if (this._isTextRangeOnlyWhitespace(startPosition, endPosition) || this._isTextRangeOnlyClosingBrace(startPosition, endPosition)) > > > - return null; > > > > Should we keep the _isTextRangeOnlyClosingBrace check? > > I don't think it's necessary. It shouldn't affect the UI in any way with the > risk that it complicates the code and also uses more computation resources > because that regexp will execute every time because if a range is white > space only, we won't cache it. read: ... the UI in any way and has the risk ...
Saam Barati
Comment 6 2015-01-29 16:50:35 PST
(In reply to comment #3) > Comment on attachment 245635 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245635&action=review > > > Source/WebInspectorUI/UserInterface/Controllers/BasicBlockAnnotator.js:-106 > > - if (this._isTextRangeOnlyWhitespace(startPosition, endPosition) || this._isTextRangeOnlyClosingBrace(startPosition, endPosition)) > > - return null; > > Should we keep the _isTextRangeOnlyClosingBrace check? I totally misread this as keep _isTextRangeOnlyWhitespace. We do want to keep the _isTextRangeOnlyClosingBrace check.
Saam Barati
Comment 7 2015-01-29 20:03:16 PST
Note You need to log in before you can comment on or make changes to this bug.