[webkit-reviews] review granted: [Bug 193147] Web Inspector: Include `globalThis` in default JavaScript completions : [Attachment 358336] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 193147: Web Inspector: Include `globalThis` in default JavaScript
completions
https://bugs.webkit.org/show_bug.cgi?id=193147

Attachment 358336: [PATCH] Proposed Fix

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




--- Comment #2 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 358336
  --> https://bugs.webkit.org/attachment.cgi?id=358336
[PATCH] Proposed Fix

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

r=me

Is `globalThis` something we'd also want to always show in the Scope chain when
paused?

>
Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.
js:661
>	       "return", "static", "super", "switch", "this", "throw", "true",
"try",

Should we be including `"globalThis"` in `allKeywords` as well?

Ditto (727) (I can't comment on it because it's outside of this diff's range)

>
Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.
js:664
> +	   var valueKeywords = ["false", "Infinity", "NaN", "null", "this",
"true", "undefined", "globalThis"];

`"globalThis"` should be placed before `"Infinity"` if we want to keep it
alphabetized.

NIT: considering how small this patch is, I wouldn't be against a bit of
refactoring (e.g. `.keySet()` to `new Set()`)