[webkit-reviews] review granted: [Bug 234575] Uncaught Exception: Cannot step over because debugger is not paused : [Attachment 448546] Patch v1.1

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 234575: Uncaught Exception: Cannot step over because debugger is not paused
https://bugs.webkit.org/show_bug.cgi?id=234575

Attachment 448546: Patch v1.1

https://bugs.webkit.org/attachment.cgi?id=448546&action=review




--- Comment #5 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 448546
  --> https://bugs.webkit.org/attachment.cgi?id=448546
Patch v1.1

View in context: https://bugs.webkit.org/attachment.cgi?id=448546&action=review

r=me with one thing to fix :)

> Source/WebInspectorUI/UserInterface/Base/Main.js:1687
> +    WI.stepOverKeyboardShortcut.disabled = !WI.debuggerManager.paused;

NIT: Can we `let paused = WI.debuggerManager.paused;` to avoid the repeated
logic?	Alternatively, you could make `WI._updateDebuggerKeyboardShortcuts`
take a parameter `paused` and hardcode the value at each callsite (e.g. we know
that the debugger is not paused inside `WI._debuggerDidResume` so there's kinda
no reason to re-check `WI.debuggerManager.paused`, tho it would be a nice
`console.assert` just to be sure).

> Source/WebInspectorUI/UserInterface/Base/Main.js:1690
> +    WI.pauseOrResumeAlternateKeyboardShortcut.disabled =
!WI.debuggerManager.paused;

I dont think this should ever be disabled since it works both when and not
paused.