[webkit-reviews] review granted: [Bug 21856] Need a way to store and retrieve preferences for the Web Inspector : [Attachment 24642] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Timothy Hatcher
<timothy at hatcher.name>'s request for review:
Bug 21856: Need a way to store and retrieve preferences for the Web Inspector
https://bugs.webkit.org/show_bug.cgi?id=21856

Attachment 24642: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=24642&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   typedef enum {
> +	       NoType, StringType, StringVectorType, DoubleType, IntegerType,
BooleanType
> +	   } Type;

You should just use "enum Type { }" syntax.

> +	   Setting(const Setting& o)
> +	       : m_type(o.m_type)
> +	       , m_string(o.m_string)
> +	       , m_stringVector(o.m_stringVector)
> +	       , m_simpleContent(o.m_simpleContent)
> +	   {
> +	   }
> +
> +	   Setting& operator=(const Setting& o)
> +	   {
> +	       m_type = o.m_type;
> +	       m_string = o.m_string;
> +	       m_stringVector = o.m_stringVector;
> +	       m_simpleContent = o.m_simpleContent;
> +	       return *this;
> +	   }

These are the same as what the compiler generates if you don't declare the
operator at all. Just leave these out and it should work the same.

> @@ -405,6 +405,10 @@ public:
>      virtual void highlight(Node*) {};
>      virtual void hideHighlight() {};
>      virtual void inspectedURLChanged(const String& newURL) {};
> +
> +    virtual void populateSetting(const String& key,
InspectorController::Setting&) {};
> +    virtual void storeSetting(const String& key, const
InspectorController::Setting&) {};
> +    virtual void removeSetting(const String& key) {};

All these semicolons at the ends of lines after {} are unneeded and should be
omitted.

> +    // FIXME: this can be shared between Mac and Windows, this is currently
copied code.

Why not make a shared source file and do this now? Is there some obstacle?

r=me