On Wed, Feb 25, 2009 at 10:53 PM, Dennis Kasprzyk < [email protected]> wrote:
> The patch look fine and I like the idea. But could you add an error message > to each of this functions if the types don't match. I'm afraid here that c++ > picks up the wrong function and we get some bugs that will be hard to > localize. With a error message we will at least know what to look for. > > I redid the patch to just call the original explicit get functions instead of copy/pasting the code, and added the check to the explicit functions (probably nicer to have it consistently warn, regardless of whether using conversion operators or explicit functions). I have attached both patches (and pasted them below, for convenience) Joel. >From 7fedcda2218dc1839411cdac76cbebda0a17dcdd Mon Sep 17 00:00:00 2001 From: Joel Bosveld <[email protected]> Date: Thu, 26 Feb 2009 20:35:26 +0900 Subject: [PATCH] Add conversion operators to CompOption::Value --- include/core/option.h | 11 ++++++++++ src/option.cpp | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 0 deletions(-) diff --git a/include/core/option.h b/include/core/option.h index 0ee362e..b9f47c5 100644 --- a/include/core/option.h +++ b/include/core/option.h @@ -98,6 +98,17 @@ class CompOption { bool operator!= (const Value& val); Value & operator= (const Value &val); + operator bool (); + operator int (); + operator float(); + operator unsigned short * (); + operator CompString (); + operator CompMatch & (); + operator CompAction & (); + operator CompAction * (); + operator Type (); + operator Vector & (); + private: PrivateValue *priv; }; diff --git a/src/option.cpp b/src/option.cpp index 66b76cb..c8fcd2f 100644 --- a/src/option.cpp +++ b/src/option.cpp @@ -250,6 +250,56 @@ CompOption::Value::list () return priv->list; } +CompOption::Value::operator bool () +{ + return b(); +} + +CompOption::Value::operator int () +{ + return i(); +} + +CompOption::Value::operator float() +{ + return f(); +} + +CompOption::Value::operator unsigned short * () +{ + return c(); +} + +CompOption::Value::operator CompString () +{ + return s(); +} + +CompOption::Value::operator CompMatch & () +{ + return match(); +} + +CompOption::Value::operator CompAction & () +{ + return action(); +} + +CompOption::Value::operator CompAction * () +{ + return &action(); +} + +CompOption::Value::operator Type () +{ + return listType(); +} + +CompOption::Value::operator Vector & () +{ + return list(); +} + bool CompOption::Value::operator== (const CompOption::Value &val) { -- 1.6.0.6 >From 4bf3017c8c53e139ebb307bc501ab8f6e2ea68f6 Mon Sep 17 00:00:00 2001 From: Joel Bosveld <[email protected]> Date: Thu, 26 Feb 2009 20:36:51 +0900 Subject: [PATCH] Warn if attempting to get wrong type from CompOption::Value --- src/option.cpp | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/src/option.cpp b/src/option.cpp index c8fcd2f..8779244 100644 --- a/src/option.cpp +++ b/src/option.cpp @@ -190,7 +190,10 @@ bool CompOption::Value::b () { if (priv->type != CompOption::TypeBool) + { + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a bool"); return false; + } return priv->value.b; } @@ -198,7 +201,10 @@ int CompOption::Value::i () { if (priv->type != CompOption::TypeInt) + { + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not an int"); return 0; + } return priv->value.i; } @@ -206,7 +212,10 @@ float CompOption::Value::f () { if (priv->type != CompOption::TypeFloat) + { + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a float"); return 0.0; + } return priv->value.f; } @@ -216,37 +225,54 @@ unsigned short* CompOption::Value::c () { if (priv->type != CompOption::TypeColor) + { + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a color"); return reinterpret_cast<unsigned short *> (&defaultColor); + } return priv->value.c; } CompString CompOption::Value::s () { + if (priv->type != CompOption::TypeString) + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a string"); return priv->string; } CompMatch & CompOption::Value::match () { + if (priv->type != CompOption::TypeMatch) + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a match"); return priv->match; } CompAction & CompOption::Value::action () { + if (priv->type != CompOption::TypeAction && + priv->type != CompOption::TypeKey && + priv->type != CompOption::TypeButton && + priv->type != CompOption::TypeEdge && + priv->type != CompOption::TypeBell) + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not an action"); return priv->action; } CompOption::Type CompOption::Value::listType () { + if (priv->type != CompOption::TypeList) + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a list"); return priv->listType; } CompOption::Value::Vector & CompOption::Value::list () { + if (priv->type != CompOption::TypeList) + compLogMessage("core", CompLogLevelWarn, "CompOption::Value not a list"); return priv->list; } -- 1.6.0.6 > > Dennis > > > Am Dienstag 24 Februar 2009 15:18:47 schrieb Joel Bosveld: > > > Adds conversion operators to CompOption::Value so we can just write > > option->value (), rather than e.g. option->value (). s() > > > > Assuming that such a change would be commited, I am not sure if the > > CompAction* is correct. I added this while testing the changes with the > > move plugin, as this was the only way I could get it to work - apart from > > the 'old' way - otherwise it would complain about invalid conversion from > > int or invalid conversion from CompValue*, depending on how I changed > that > > part in move. > > > > Joel. > > > > ------ > > > > From ba9be143d61644c8db0b6161f4c3f8db6510d96c Mon Sep 17 00:00:00 2001 > > From: Joel Bosveld <[email protected]> > > Date: Tue, 24 Feb 2009 22:43:50 +0900 > > Subject: [PATCH] Add conversion operators to CompOption::Value > > > > --- > > include/core/option.h | 11 +++++++++ > > src/option.cpp | 58 > > +++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 69 insertions(+), 0 deletions(-) > > > > diff --git a/include/core/option.h b/include/core/option.h > > index 0ee362e..b9f47c5 100644 > > --- a/include/core/option.h > > +++ b/include/core/option.h > > @@ -98,6 +98,17 @@ class CompOption { > > bool operator!= (const Value& val); > > Value & operator= (const Value &val); > > > > + operator bool (); > > + operator int (); > > + operator float(); > > + operator unsigned short * (); > > + operator CompString (); > > + operator CompMatch & (); > > + operator CompAction & (); > > + operator CompAction * (); > > + operator Type (); > > + operator Vector & (); > > + > > private: > > PrivateValue *priv; > > }; > > diff --git a/src/option.cpp b/src/option.cpp > > index 66b76cb..4705746 100644 > > --- a/src/option.cpp > > +++ b/src/option.cpp > > @@ -250,6 +250,64 @@ CompOption::Value::list () > > return priv->list; > > } > > > > +CompOption::Value::operator bool () > > +{ > > + if (priv->type != CompOption::TypeBool) > > + return false; > > + return priv->value.b; > > +} > > + > > +CompOption::Value::operator int () > > +{ > > + if (priv->type != CompOption::TypeInt) > > + return 0; > > + return priv->value.i; > > +} > > + > > +CompOption::Value::operator float() > > +{ > > + if (priv->type != CompOption::TypeFloat) > > + return 0.0; > > + return priv->value.f; > > +} > > + > > +CompOption::Value::operator unsigned short * () > > +{ > > + if (priv->type != CompOption::TypeColor) > > + return reinterpret_cast<unsigned short *> (&defaultColor); > > + return priv->value.c; > > +} > > + > > +CompOption::Value::operator CompString () > > +{ > > + return priv->string; > > +} > > + > > +CompOption::Value::operator CompMatch & () > > +{ > > + return priv->match; > > +} > > + > > +CompOption::Value::operator CompAction & () > > +{ > > + return priv->action; > > +} > > + > > +CompOption::Value::operator CompAction * () > > +{ > > + return &priv->action; > > +} > > + > > +CompOption::Value::operator Type () > > +{ > > + return priv->listType; > > +} > > + > > +CompOption::Value::operator Vector & () > > +{ > > + return priv->list; > > +} > > + > > bool > > CompOption::Value::operator== (const CompOption::Value &val) > > { > > > > > _______________________________________________ > Dev mailing list > [email protected] > http://lists.compiz-fusion.org/mailman/listinfo/dev > >
0001-Add-conversion-operators-to-CompOption-Value.patch
Description: Binary data
0002-Warn-if-attempting-to-get-wrong-type-from-CompOption.patch
Description: Binary data
_______________________________________________ Dev mailing list [email protected] http://lists.compiz-fusion.org/mailman/listinfo/dev
