Bug 173355 - [GTK] Use API::InjectedBundle::EditorClient in WebKitWebEditor
Summary: [GTK] Use API::InjectedBundle::EditorClient in WebKitWebEditor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2017-06-14 01:11 PDT by Carlos Garcia Campos
Modified: 2017-06-14 16:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.94 KB, patch)
2017-06-14 01:13 PDT, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-06-14 01:11:54 PDT
Instead of the C API.
Comment 1 Carlos Garcia Campos 2017-06-14 01:13:08 PDT
Created attachment 312869 [details]
Patch
Comment 2 Zan Dobersek 2017-06-14 02:40:58 PDT
Comment on attachment 312869 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebEditor.cpp:85
> +    void didChangeSelection(WebPage&, StringImpl* /*notificationName*/) override

Consider UNUSED_PARAM(notificationName), or just not listing the argument name.
Comment 3 Carlos Garcia Campos 2017-06-14 03:49:29 PDT
Committed r218251: <http://trac.webkit.org/changeset/218251>
Comment 4 Darin Adler 2017-06-14 07:39:19 PDT
Comment on attachment 312869 [details]
Patch

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

>> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebEditor.cpp:85
>> +    void didChangeSelection(WebPage&, StringImpl* /*notificationName*/) override
> 
> Consider UNUSED_PARAM(notificationName), or just not listing the argument name.

For what it’s worth, I consider commenting out the name to be better than UNUSED_PARAM. I see UNUSED_PARAM as a necessary evil that we try to avoid, since it doesn’t even work -- it silences the warning, but doesn’t prevent use of the thing it marks as "unused"! But maybe we don’t have consensus on that.
Comment 5 Michael Catanzaro 2017-06-14 08:48:35 PDT
UNUSED_PARAM should be used in cases where the parameter is used in one preprocessor conditional but not another. Otherwise, it's better to either comment out or omit the name.
Comment 6 Darin Adler 2017-06-14 16:34:18 PDT
(In reply to Michael Catanzaro from comment #5)
> UNUSED_PARAM should be used in cases where the parameter is used in one
> preprocessor conditional but not another. Otherwise, it's better to either
> comment out or omit the name.

Yes, I agree with that.