Bug 141011 – Web Inspector: Make BasicBlockAnnotator lessen the saturation of syntax highlighting instead of graying out unexecuted code

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Saam Barati]

This provides a nicer user experience.

[Radar WebKit Bug Importer]

<rdar://problem/19638077>

[Saam Barati]

Created attachment 245635 [details]
patch

[Timothy Hatcher]

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]

(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]

(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]

(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]

landed in:
http://trac.webkit.org/changeset/179390