Summary: | Web Inspector: modernize InjectedScript a bit | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||
Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, bburg, ews-watchlist, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, saam, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
BJ Burg
2017-12-04 11:27:29 PST
Created attachment 328384 [details]
Patch
Attachment 328384 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:115: out_wasThrown is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:115: out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:142: out_wasThrown is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:239: out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:96: out_resultObject is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:96: out_wasThrown is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:96: out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:830: out_wasThrown is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:830: out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 9 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Build Bot from comment #2) > Attachment 328384 [details] did not pass style-queue: > > > ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:115: > out_wasThrown is incorrectly named. Don't use underscores in your identifier > names. [readability/naming/underscores] [4] > ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:115: > out_savedResultIndex is incorrectly named. Don't use underscores in your > identifier names. [readability/naming/underscores] [4] > ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:142: > out_wasThrown is incorrectly named. Don't use underscores in your identifier > names. [readability/naming/underscores] [4] > ERROR: Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:239: > out_savedResultIndex is incorrectly named. Don't use underscores in your > identifier names. [readability/naming/underscores] [4] > ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:96: > out_resultObject is incorrectly named. Don't use underscores in your > identifier names. [readability/naming/underscores] [4] > ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:96: > out_wasThrown is incorrectly named. Don't use underscores in your identifier > names. [readability/naming/underscores] [4] > ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:96: > out_savedResultIndex is incorrectly named. Don't use underscores in your > identifier names. [readability/naming/underscores] [4] > ERROR: > Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:830: > out_wasThrown is incorrectly named. Don't use underscores in your identifier > names. [readability/naming/underscores] [4] > ERROR: > Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:830: > out_savedResultIndex is incorrectly named. Don't use underscores in your > identifier names. [readability/naming/underscores] [4] > Total errors found: 9 in 8 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. Style bot can go jump in a lake. The code reads much, much better with the out_ prefix. Comment on attachment 328384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328384&action=review > Source/JavaScriptCore/ChangeLog:10 > + If there is only one out-parameter and a void return type, then return the value. I think we should return a std::pair or a std::tuple instead of having out_-prefixed variable names. Comment on attachment 328384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328384&action=review > Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:128 > + out_resultObject = BindingTraits<Protocol::Runtime::RemoteObject>::runtimeCast(resultObject); Is "out_" prefix the style we use elsewhere? No a fan. > Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:127 > + // FIXME: remove this bridging code when generated protocol commands no longer use OptOutput<T>. Is there a bug you can reference? > Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:256 > + // FIXME: remove this bridging code when generated protocol commands no longer use OptOutput<T>. Ditto. Comment on attachment 328384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328384&action=review >> Source/JavaScriptCore/ChangeLog:10 >> + If there is only one out-parameter and a void return type, then return the value. > > I think we should return a std::pair or a std::tuple instead of having out_-prefixed variable names. This is the longer term goal for where it makes sense, along with using Expected<T, ErrorString> rather than ErrorString as a in-out param everywhere. But my first-order goal is to remove OptOutput and use std::optional in generated signatures. I'm not completely sold on using Tuples instead of out-params in generated protocol code, as it will make it a lot more annoying to use for little gain. >> Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:127 >> + // FIXME: remove this bridging code when generated protocol commands no longer use OptOutput<T>. > > Is there a bug you can reference? I'll add one. It's the next thing I want to clean up. Committed r225804: <https://trac.webkit.org/changeset/225804> |