Moved UI talk to here.
> I'm a much bigger fan of small widgets in gutters (left side) ... I’m willing to explore that option. I have used Adobe Theseus (https://github.com/adobe-research/theseus#theseus) which adds call counters for every function. I find it very helpful to see numbers changing in realtime. I haven’t used any software that adds a heat map overlay on top of code. I think it could be useful to give a quick glance of what happened. We already gray out unexecuted code and I find it very helpful. Heat maps could be an extension to that. I’ll make two prototypes: gutter highlight and heat maps. I’ll post them here for anyone to play around.
(In reply to comment #1) > > I'm a much bigger fan of small widgets in gutters (left side) ... > > I’m willing to explore that option. > > I have used Adobe Theseus > (https://github.com/adobe-research/theseus#theseus) which adds call counters > for every function. I find it very helpful to see numbers changing in > realtime. Yeah, I love Tom's work on Theseus. We gotta take that stuff and ship it! I am concerned that real numbers won't scale once you get to hot loops. The widely varying input domain also make it difficult to write a generic binning function. A nice thing about gutters is that it provides a good place to hover for more information, either in a popover or by adding underlines to different subexpressions on the hovered line. This is the most relevant academic project, which I think has a cool idea and a terrible visual encoding: http://scg.unibe.ch/research/senseo > I haven’t used any software that adds a heat map overlay on top of code. I > think it could be useful to give a quick glance of what happened. We already > gray out unexecuted code and I find it very helpful. Heat maps could be an > extension to that. In the other bug, I believe Oliver was referring to colors in the rows/cells of a profiler. Color seems fine there since a user isn't trying to comprehend what the words mean as much as find the heaviest line.
<rdar://problem/21475874>
We need to update CodeMirror to 5.3+ to be able to add inline CSS via addCssToTextRange. Our current version of CodeMirror only allows to add CSS class names.
Created attachment 260272 [details] WIP Saam's patch from https://bugs.webkit.org/show_bug.cgi?id=146099#c14 + UI changes.
Created attachment 260273 [details] [Image] With the patch applied Unexecuted code highlighted with a dim purple background so I could see the ranges even if they're just a white space. This is for debugging only. I'll revert to text opacity when the patch is ready.
It isn't clear what the colors mean in the screenshot.
Created attachment 264464 [details] WIP Screenshot: https://cldup.com/5Zjt-ccMFl.png Changes from the previous WIP: - Execution counts The idea is to show execution counts on the line gutter. This is inspired by Theseus (https://github.com/adobe-research/theseus). Currently, execution counts are shown for all blocks. I'll change it to show only for functions and loops in the future. Execution counts are often shown one line before the line they were supposed to be in because white space isn't handled properly. I'll fix that. - Efficient rendering. Only modified blocks are re-rendered. The ranges from the backend may intersect and I implemented flattening. The rule is that the inner ranges always win over the outer ones. For example, given two ranges: {start: 30, end: 50, executionCount: 0} {start: 0, end: 100, executionCount: 1} The flattening algorithm returns: {start: 0, end: 29, executionCount: 1} {start: 30, end: 50, executionCount: 0} {start: 51, end: 100, executionCount: 1} I'll write tests for this later. Lots of CodeMirro-related visual bugs that need to be fixed: — Hot code's background overlays the selection, making the selection invisible – Execution counts' background overlaps the breakpoints' background — White patches around the type pills — Many issues with white space highlighting
Comment on attachment 264464 [details] WIP I realize that this is a WIP patch, but here are some things I noticed while reading through it. View in context: https://bugs.webkit.org/attachment.cgi?id=264464&action=review > Source/WebInspectorUI/UserInterface/Base/Utilities.js:476 > + var result = []; let instead of var > Source/WebInspectorUI/UserInterface/Base/Utilities.js:482 > + if (comparator(left[indexLeft], right[indexRight]) < 0) To make this more extensible, you may consider moving the "< 0" into the comparator > Source/WebInspectorUI/UserInterface/Main.html:209 > + <script src="External/Chroma/chroma.js"></script> We already have a WebInspector.Color, so is there any way to try to merge the two classes? I'm not sure why exactly it is necessary to use an external library for just a few calls, but if you have a good reason then its fine by me (although please explain it). > Source/WebInspectorUI/UserInterface/Views/TextEditor.js:452 > + var doc = this._codeMirror.getDoc(); let instead of var > Source/WebInspectorUI/UserInterface/Views/TextEditor.js:462 > + m.className = "CodeMirror-linenumber gutter-execution-count"; classList.add() > Source/WebInspectorUI/UserInterface/Views/TextEditor.js:468 > + if (text !== undefined) Shouldn't this just be if (text && text.length)