* SUMMARY Inline Error / Warning message for Issues. Currently we highlight lines red/yellow for errors/warnings but we should show the warning itself inline. Likewise, they should work with pretty printing, since right now issues don't!
<rdar://problem/19525240>
Created attachment 248309 [details] [PATCH] Proposed Fix
Created attachment 248310 [details] [IMAGE] Pre-Pretty-Printing
Created attachment 248311 [details] [IMAGE] Post-Pretty-Printing
Created attachment 248312 [details] [IMAGE] Expanded Errors/Warnings Note: Only truncated issues, or lines with multiple issues, are expandable. This cuts down on the awkwardness. Maybe we would want to do this when issues overlap content, but I didn't go that far. The developer can just resize.
Comment on attachment 248309 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=248309&action=review > Source/WebInspectorUI/UserInterface/Models/LineWidget.js:26 > +WebInspector.LineWidget = function(codeMirrorLineWidget, widgetElement) SourceCodeLineWidget? Something about the short class name makes me think it needs to be longer. > Source/WebInspectorUI/UserInterface/Models/LineWidget.js:42 > + get codeMirrorLineWidget() Makes me wonder if this class should have CodeMirror in the class name. But we don't use CodeMirror in this file so hmm... > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.css:75 > + padding-right: calc(9px + 3px); Just flatten to 12px? > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:811 > + // FIXME: Issue should have a SourceCodeLocation. Yep! > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:899 > + errorsCount++; Prefix ++. > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:901 > + warningsCount++; Ditto. > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:1217 > + for (var [lineNumber, widget] of this._widgetMap) Maybe use .values() or [, widget]? > Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:1230 > + for (var i = 0; i < issues.length; ++i) { > + var issue = issues[i]; for (..of..)
(In reply to comment #6) > Comment on attachment 248309 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=248309&action=review > > > Source/WebInspectorUI/UserInterface/Models/LineWidget.js:26 > > +WebInspector.LineWidget = function(codeMirrorLineWidget, widgetElement) > > SourceCodeLineWidget? Something about the short class name makes me think it > needs to be longer. > > > Source/WebInspectorUI/UserInterface/Models/LineWidget.js:42 > > + get codeMirrorLineWidget() > > Makes me wonder if this class should have CodeMirror in the class name. But > we don't use CodeMirror in this file so hmm... Hmm, yeah. This is mostly modeled after Models/TextMarker.js. Both should move to CodeMirror names eventually. For now I'll leave as is because it is nice that they are simple.
<http://trac.webkit.org/changeset/181306>