Created attachment 278426 [details] Screenshot We should not show the console.log (etc) location for direct calls when a result has not been returned yet. This would let the location show up for async logs later still.
<rdar://problem/26177460>
Created attachment 304691 [details] Patch
Created attachment 304692 [details] [Image] After Patch is applied
I'd argue we drop the whole right hand thing, not jus the location. I think it is only useful when it is a function call defined in the console that gets called out of line. Like: > setTimeout(100, function() { console.log("Here") }) < undefined L: "Here" [S] Console Evaluation — ConsoleEvaluation:1:13 Nothing else would have the [S] Console Evaluation line.
Comment on attachment 304691 [details] Patch Subtitle should only be created if we're linking to a source location. Also the cursor when hovering Console Evaluation should be the default, instead of a pointer (hand).
(In reply to comment #4) > I'd argue we drop the whole right hand thing, not jus the location. I think > it is only useful when it is a function call defined in the console that > gets called out of line. > > Like: > > > setTimeout(100, function() { console.log("Here") }) > < undefined > L: "Here" [S] Console Evaluation — ConsoleEvaluation:1:13 > > > Nothing else would have the [S] Console Evaluation line. Let's drop it!
Created attachment 304699 [details] Patch
Comment on attachment 304699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304699&action=review > Source/WebInspectorUI/UserInterface/Models/CallFrame.js:266 > + return new WebInspector.CallFrame(target, id, sourceCodeLocation, functionName, thisObject, scopeChain, nativeCode, programCode, isTailDeleted, isConsoleEvaluation); I think this change is unnecessary. Why not have CallFrame determine `isConsoleEvaluation` in its constructor?
Comment on attachment 304699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304699&action=review > Source/WebInspectorUI/UserInterface/Models/CallFrame.js:259 > programCode = true; This can also move into CallFrame's constructor, guaranteeing that `isConsoleEvaluation`, `functionName`, and `programCode` are internally consistent.
Created attachment 304717 [details] Patch
Comment on attachment 304717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304717&action=review Looks good, r=me with a nit. > Source/WebInspectorUI/UserInterface/Models/CallFrame.js:52 > + this._isConsoleEvaluation = isConsoleEvaluation || false; Don't need to OR with false, `isConsoleEvaluation` is a boolean by definition.
Created attachment 304736 [details] Patch
Comment on attachment 304736 [details] Patch Clearing flags on attachment: 304736 Committed r214093: <http://trac.webkit.org/changeset/214093>
All reviewed patches have been landed. Closing bug.