Merge lp:~thomas-voss/compiz-core/fix-globals-simplified into lp:compiz-core/0.9.5

[Sam Spilsbury]

Move the globals definitions into a separate file

[Alan Griffiths (alan-griffiths)]

There seems to be a lot shown in the diff that isn't about moving global variables. (That bit seems fine.)

[Daniel van Vugt (vanvugt)]

Please remove all the changes unrelated to the globals clean-up in question.

It looks like lots of them crept in from:
https://code.launchpad.net/~thomas-voss/compiz-core/CompizOptionRework/+merge/87714

review: Needs Fixing

[lp:~thomas-voss/compiz-core/fix-globals-simplified]

2922. By Thomas Voß on 2012-01-19

Re-enabled command-line flag --use-root-window and defined the variable in main.cpp.

Unmerged revisions

2913. By Daniel van Vugt on 2012-01-16

Fixes focus being on the wrong window after viewport changes (LP: #896762)
Merged from lp:~smspillaz/compiz-core/fix_896762

=== modified file 'include/core/action.h'
--- include/core/action.h	2010-11-09 14:13:19 +0000
+++ include/core/action.h	2012-01-18 14:51:30 +0000
@@ -154,7 +154,7 @@
 
 	void copyState (const CompAction &action);
 
-	bool operator== (const CompAction& val);
+	bool operator== (const CompAction& val) const;
 	CompAction & operator= (const CompAction &action);
 
 	bool keyFromString (const CompString &str);

=== modified file 'include/core/option.h'
--- include/core/option.h	2011-10-31 13:51:00 +0000
+++ include/core/option.h	2012-01-18 14:51:30 +0000
@@ -29,11 +29,15 @@
 #define _COMPOPTION_H
 
 #include <core/string.h>
+
+#include <boost/variant.hpp>
+
 #include <vector>
 
 class PrivateOption;
 class PrivateValue;
 class PrivateRestriction;
+
 class CompAction;
 class CompMatch;
 class CompScreen;
@@ -54,12 +58,12 @@
 	    TypeString,
 	    TypeColor,
 	    TypeAction,
+	    TypeMatch,
+	    TypeList,
 	    TypeKey,
 	    TypeButton,
 	    TypeEdge,
 	    TypeBell,
-	    TypeMatch,
-	    TypeList,
 	    /* internal use only */
 	    TypeUnset
 	} Type;
@@ -69,61 +73,105 @@
 	 */
 	class Value {
 	    public:
+
+	  typedef boost::variant<
+	      bool,
+	      int,
+	      float,
+	      CompString,
+	      unsigned short*,
+	      boost::recursive_wrapper<CompAction>,
+	      boost::recursive_wrapper<CompMatch>,
+	      boost::recursive_wrapper<std::vector<Value> >
+	    > variant_type;
+
 		typedef std::vector<Value> Vector;
 
 	    public:
-		Value ();
-		Value (const Value &);
-		Value (const bool b);
-		Value (const int i);
-		Value (const float f);
-		Value (const unsigned short *color);
-		Value (const CompString& s);
-		Value (const char *s);
-		Value (const CompMatch& m);
-		Value (const CompAction& a);
-		Value (Type type, const Vector& l);
-		~Value ();
-
-		Type type () const;
-
-		void set (const bool b);
-		void set (const int i);
-		void set (const float f);
-		void set (const unsigned short *color);
-		void set (const CompString& s);
-		void set (const char *s);
-		void set (const CompMatch& m);
-		void set (const CompAction& a);
-		void set (Type type, const Vector& l);
-
-		bool               b ();
-		int                i ();
-		float              f ();
-		unsigned short*    c ();
-		CompString         s ();
-		CompMatch &        match ();
-		CompAction &       action ();
-		Type               listType ();
-		Vector &           list ();
-
-		bool operator== (const Value& val);
-		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 & ();
+		Value () : mListType(TypeUnset)
+		{
+		}
+
+		template<typename T>
+		Value( const T & t ) : mListType(TypeUnset),
+		mValue(t)
+		{
+		}
+
+		~Value();
+
+		Type
+		type () const
+		{
+		    return static_cast<Type>(mValue.which());
+		}
+
+		Type
+		listType () const
+		{
+		    return mListType;
+		}
+
+		template<typename T>
+		void set (const T & t)
+		{
+		    mValue = t;
+		}
+
+		template<typename T>
+		const T & get () const
+		{
+		    return boost::get<T> (mValue);
+		}
+
+		void
+		set (Type t, const Vector & v);
+
+		bool
+		b () const;
+
+		int
+		i () const;
+
+		float
+		f () const;
+
+		unsigned short*
+		c () const;
+
+		const CompString &
+		s () const;
+
+		CompString &
+		s ();
+
+		const CompMatch &
+		match () const;
+
+		CompMatch &
+		match ();
+
+		const CompAction &
+		action () const;
+
+		CompAction &
+		action ();
+
+		const Vector &
+		list () const;
+
+		Vector &
+		list ();
+
+		bool
+		operator== (const Value & rhs) const;
+
+		bool
+		operator!= (const Value & rhs) const;
 
 	    private:
-		PrivateValue *priv;
+		Type mListType;
+		variant_type mValue;
 	};
 
 	/**

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2012-01-12 13:44:07 +0000
+++ src/CMakeLists.txt	2012-01-18 14:51:30 +0000
@@ -59,10 +59,10 @@
     ${CORE_MOD_LIBRARY_DIRS}
 )
 
-add_executable (compiz
-    ${CMAKE_CURRENT_SOURCE_DIR}/region.cpp
+add_library (compiz_core
+	${CMAKE_CURRENT_SOURCE_DIR}/global.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/region.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/atoms.cpp
-    ${CMAKE_CURRENT_SOURCE_DIR}/main.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/actions.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/screen.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/window.cpp
@@ -86,6 +86,10 @@
     ${_bcop_sources}
 )
 
+add_executable (compiz
+    ${CMAKE_CURRENT_SOURCE_DIR}/main.cpp
+)
+
 # workaround for build race
 add_dependencies (compiz core-xml-file)
 
@@ -94,17 +98,33 @@
 	      PROPERTY CORE_MOD_LIBRARIES)
 
 target_link_libraries (
-    compiz ${COMPIZ_LIBRARIES} 
-    
-    m 
-    pthread 
-    dl
-    
-    compiz_string
-    compiz_timer
-    compiz_logmessage
-    compiz_pluginclasshandler
-#    ${CORE_MOD_LIBRARIES}
+    compiz_core 
+    
+    ${COMPIZ_LIBRARIES} 
+    
+    m 
+    pthread 
+    dl
+    
+    compiz_string
+    compiz_timer
+    compiz_logmessage
+    compiz_pluginclasshandler
+)
+
+target_link_libraries (
+    compiz 
+    compiz_core
+    ${COMPIZ_LIBRARIES} 
+    
+    m 
+    pthread 
+    dl
+    
+    compiz_string
+    compiz_timer
+    compiz_logmessage
+    compiz_pluginclasshandler
 )
 
 install (
@@ -112,4 +132,6 @@
     DESTINATION ${COMPIZ_DESTDIR}${exec_prefix}
 )
 
+add_subdirectory(tests)
+
 enable_coverage_report( TARGETS compiz )

=== modified file 'src/action.cpp'
--- src/action.cpp	2011-10-31 13:51:00 +0000
+++ src/action.cpp	2012-01-18 14:51:30 +0000
@@ -455,7 +455,7 @@
 }
 
 bool
-CompAction::operator== (const CompAction& val)
+CompAction::operator== (const CompAction& val) const
 {
     if (priv->state != val.priv->state)
 	return false;

=== added file 'src/global.cpp'
--- src/global.cpp	1970-01-01 00:00:00 +0000
+++ src/global.cpp	2012-01-18 14:51:30 +0000
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Novell, Inc. not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Novell, Inc. makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * NOVELL, INC. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL NOVELL, INC. BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author: Thomas Voss <[e-mail]>
+ */
+
+#include <list>
+
+#include "core/string.h"
+
+class CompWindow;
+
+char *programName;
+char **programArgv;
+int  programArgc;
+
+char *backgroundImage = NULL;
+
+bool shutDown = false;
+bool restartSignal = false;
+
+CompWindow *lastFoundWindow = 0;
+
+bool replaceCurrentWm = false;
+bool indirectRendering = false;
+bool noDetection = false;
+bool useDesktopHints = false;
+bool debugOutput = false;
+bool useCow = true;
+
+std::list <CompString> initialPlugins;
+
+unsigned int pluginClassHandlerIndex = 0;

=== modified file 'src/main.cpp'
--- src/main.cpp	2011-10-31 13:51:00 +0000
+++ src/main.cpp	2012-01-18 14:51:30 +0000
@@ -38,28 +38,6 @@
 #include "privatescreen.h"
 #include "privatestackdebugger.h"
 
-char *programName;
-char **programArgv;
-int  programArgc;
-
-char *backgroundImage = NULL;
-
-bool shutDown = false;
-bool restartSignal = false;
-
-CompWindow *lastFoundWindow = 0;
-
-bool replaceCurrentWm = false;
-bool indirectRendering = false;
-bool noDetection = false;
-bool useDesktopHints = false;
-bool debugOutput = false;
-bool useCow = true;
-
-std::list <CompString> initialPlugins;
-
-unsigned int pluginClassHandlerIndex = 0;
-
 void
 CompManager::usage ()
 {
@@ -72,7 +50,6 @@
 	    "[--bg-image PNG] "
 	    "[--no-detection] "
 	    "[--keep-desktop-hints]\n       "
-	    "[--use-root-window] "
 	    "[--debug] "
 	    "[--version] "
 	    "[--help] "
@@ -132,10 +109,6 @@
 	{
 	    useDesktopHints = true;
 	}
-	else if (!strcmp (argv[i], "--use-root-window"))
-	{
-	    useCow = false;
-	}
 	else if (!strcmp (argv[i], "--replace"))
 	{
 	    replaceCurrentWm = true;

=== modified file 'src/option.cpp'
--- src/option.cpp	2010-11-11 03:14:20 +0000
+++ src/option.cpp	2012-01-18 14:51:30 +0000
@@ -32,164 +32,14 @@
 #include <boost/foreach.hpp>
 #define foreach BOOST_FOREACH
 
+#include <core/action.h>
 #include <core/core.h>
+#include <core/match.h>
 #include <core/option.h>
 #include "privateoption.h"
 
 CompOption::Vector noOptions (0);
 
-CompOption::Value::Value () :
-    priv (new PrivateValue ())
-{
-}
-
-CompOption::Value::Value (const Value &v) :
-    priv (new PrivateValue (*v.priv))
-{
-}
-
-CompOption::Value::~Value ()
-{
-    delete priv;
-}
-
-CompOption::Value::Value (const bool b) :
-    priv (new PrivateValue ())
-{
-    set (b);
-}
-
-CompOption::Value::Value (const int i) :
-    priv (new PrivateValue ())
-{
-    set (i);
-}
-
-CompOption::Value::Value (const float f) :
-    priv (new PrivateValue ())
-{
-    set (f);
-}
-
-CompOption::Value::Value (const unsigned short *color) :
-    priv (new PrivateValue ())
-{
-    set (color);
-}
-
-CompOption::Value::Value (const CompString& s) :
-    priv (new PrivateValue ())
-{
-    set (s);
-}
-
-CompOption::Value::Value (const char *s) :
-    priv (new PrivateValue ())
-{
-    set (s);
-}
-
-
-CompOption::Value::Value (const CompMatch& m) :
-    priv (new PrivateValue ())
-{
-    set (m);
-}
-
-CompOption::Value::Value (const CompAction& a) :
-    priv (new PrivateValue ())
-{
-    set (a);
-}
-
-CompOption::Value::Value (CompOption::Type type, const Vector& l) :
-    priv (new PrivateValue ())
-{
-    set (type, l);
-}
-
-CompOption::Type
-CompOption::Value::type () const
-{
-    return priv->type;
-}
-
-void
-CompOption::Value::set (const bool b)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeBool;
-    priv->value.b = b;
-}
-
-void
-CompOption::Value::set (const int i)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeInt;
-    priv->value.i = i;
-}
-
-void
-CompOption::Value::set (const float f)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeFloat;
-    priv->value.f = f;
-}
-
-void
-CompOption::Value::set (const unsigned short *color)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeColor;
-    priv->value.c[0] = color[0];
-    priv->value.c[1] = color[1];
-    priv->value.c[2] = color[2];
-    priv->value.c[3] = color[3];
-}
-
-void
-CompOption::Value::set (const CompString& s)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeString;
-    priv->string = s;
-}
-
-void
-CompOption::Value::set (const char *s)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeString;
-    priv->string = CompString (s);
-}
-
-void
-CompOption::Value::set (const CompMatch& m)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeMatch;
-    priv->match = m;
-}
-
-void
-CompOption::Value::set (const CompAction& a)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeAction;
-    priv->action = a;
-}
-
-void
-CompOption::Value::set (CompOption::Type type, const Vector& l)
-{
-    priv->reset ();
-    priv->type = CompOption::TypeList;
-    priv->list = l;
-    priv->listType = type;
-}
-
 static bool
 checkIsAction (CompOption::Type type)
 {
@@ -207,307 +57,130 @@
     return false;
 }
 
-bool
-CompOption::Value::b ()
-{
-    if (!priv->checkType (CompOption::TypeBool))
-	return false;
-
-    return priv->value.b;
-}
-
-int
-CompOption::Value::i ()
-{
-    if (!priv->checkType (CompOption::TypeInt))
-	return 0;
-
-    return priv->value.i;
-}
-
-float
-CompOption::Value::f ()
-{
-    if (!priv->checkType (CompOption::TypeFloat))
-	return 0.0;
-
-    return priv->value.f;
-}
+
 
 static unsigned short defaultColor[4] = { 0x0, 0x0, 0x0, 0xffff};
 
-unsigned short *
-CompOption::Value::c ()
-{
-    if (!priv->checkType (CompOption::TypeColor))
-	return reinterpret_cast<unsigned short *> (defaultColor);
-
-    return priv->value.c;
-}
-
-CompString
+
+static void
+finiOptionValue (CompOption::Value &v)
+{
+    switch (v.type()) {
+	case CompOption::TypeAction:
+	case CompOption::TypeKey:
+	case CompOption::TypeButton:
+	case CompOption::TypeEdge:
+	case CompOption::TypeBell:
+	    if (v.action ().state () & CompAction::StateAutoGrab && screen)
+		screen->removeAction (&v.action ());
+	    break;
+
+	case CompOption::TypeList:
+	    foreach (CompOption::Value &val, v.list ())
+		finiOptionValue (val);
+	    break;
+
+	default:
+	    break;
+    }
+}
+
+CompOption::Value::~Value()
+{
+    finiOptionValue(*this);
+}
+
+void
+CompOption::Value::set (Type t, const CompOption::Value::Vector & v)
+{
+    mListType = t;
+    mValue = v;
+}
+
+bool
+CompOption::Value::b () const
+{
+    return boost::get<bool>(mValue);
+}
+
+int
+CompOption::Value::i () const
+{
+    return boost::get<int>(mValue);
+}
+
+float
+CompOption::Value::f () const
+{
+    return boost::get<float>(mValue);
+}
+
+unsigned short*
+CompOption::Value::c () const
+{
+    return boost::get<unsigned short*>(mValue);
+}
+
+const CompString &
+CompOption::Value::s () const
+{
+    return boost::get<CompString>(mValue);
+}
+
+CompString &
 CompOption::Value::s ()
 {
-    if (!priv->checkType (CompOption::TypeString))
-	return "";
+    return boost::get < CompString > (mValue);
+}
 
-    return priv->string;
+const CompMatch &
+CompOption::Value::match () const
+{
+    return boost::get<CompMatch>(mValue);
 }
 
 CompMatch &
 CompOption::Value::match ()
 {
-    priv->checkType (CompOption::TypeMatch);
+    return boost::get<CompMatch>(mValue);
+}
 
-    return priv->match;
+const CompAction &
+CompOption::Value::action () const
+{
+    return boost::get<CompAction>(mValue);
 }
 
 CompAction &
 CompOption::Value::action ()
 {
-    priv->checkType (priv->type);
-
-    if (!checkIsAction (priv->type))
-	compLogMessage ("core", CompLogLevelWarn,
-			"CompOption::Value not an action");
-
-    return priv->action;
+    return boost::get<CompAction>(mValue);
 }
 
-CompOption::Type
-CompOption::Value::listType ()
+// Type listType () const;
+
+const CompOption::Value::Vector &
+CompOption::Value::list () const
 {
-    priv->checkType (CompOption::TypeList);
-
-    return priv->listType;
+    return boost::get< std::vector<Value> >(mValue);
 }
 
 CompOption::Value::Vector &
 CompOption::Value::list ()
 {
-    priv->checkType (CompOption::TypeList);
-
-    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)
-{
-    if (priv->type != val.priv->type)
-	return false;
-
-    switch (priv->type)
-    {
-	case CompOption::TypeBool:
-	    return priv->value.b == val.priv->value.b;
-	    break;
-
-	case CompOption::TypeInt:
-	    return priv->value.i == val.priv->value.i;
-	    break;
-
-	case CompOption::TypeFloat:
-	    return priv->value.f == val.priv->value.f;
-	    break;
-
-	case CompOption::TypeColor:
-	    return (priv->value.c[0] == val.priv->value.c[0]) &&
-		   (priv->value.c[1] == val.priv->value.c[1]) &&
-		   (priv->value.c[2] == val.priv->value.c[2]) &&
-		   (priv->value.c[3] == val.priv->value.c[3]);
-	    break;
-
-	case CompOption::TypeString:
-	    return priv->string.compare (val.priv->string) == 0;
-	    break;
-
-	case CompOption::TypeMatch:
-	    return priv->match == val.priv->match;
-	    break;
-
-	case CompOption::TypeAction:
-	    return priv->action == val.priv->action;
-	    break;
-
-	case CompOption::TypeList:
-	    if (priv->listType != val.priv->listType)
-		return false;
-
-	    if (priv->list.size () != val.priv->list.size ())
-		return false;
-
-	    for (unsigned int i = 0; i < priv->list.size (); i++)
-		if (priv->list[i] != val.priv->list[i])
-		    return false;
-
-	    return true;
-	    break;
-
-	default:
-	    break;
-    }
-
-    return true;
-}
-
-bool
-CompOption::Value::operator!= (const CompOption::Value &val)
-{
-    return !(*this == val);
-}
-
-static void
-finiOptionValue (CompOption::Value &v,
-		 CompOption::Type  type)
-{
-    switch (type) {
-	case CompOption::TypeAction:
-	case CompOption::TypeKey:
-	case CompOption::TypeButton:
-	case CompOption::TypeEdge:
-	case CompOption::TypeBell:
-	    if (v.action ().state () & CompAction::StateAutoGrab && screen)
-		screen->removeAction (&v.action ());
-	    break;
-
-	case CompOption::TypeList:
-	    foreach (CompOption::Value &val, v.list ())
-		finiOptionValue (val, v.listType ());
-	    break;
-
-	default:
-	    break;
-    }
-}
-
-CompOption::Value &
-CompOption::Value::operator= (const CompOption::Value &val)
-{
-    if (this == &val)
-	return *this;
-
-    finiOptionValue (*this, priv->type);
-
-    delete priv;
-    priv = new PrivateValue (*val.priv);
-
-    return *this;
-}
-
-PrivateValue::PrivateValue () :
-    type (CompOption::TypeUnset),
-    string (""),
-    action (),
-    match (),
-    listType (CompOption::TypeUnset),
-    list ()
-{
-    memset (&value, 0, sizeof (ValueUnion));
-}
-
-PrivateValue::PrivateValue (const PrivateValue& p) :
-    type (p.type),
-    string (p.string),
-    action (p.action),
-    match (p.match),
-    listType (p.listType),
-    list (p.list)
-{
-    memcpy (&value, &p.value, sizeof (ValueUnion));
-}
-
-bool
-PrivateValue::checkType (CompOption::Type refType)
-{
-    if (type == CompOption::TypeUnset)
-    {
-	compLogMessage ("core", CompLogLevelWarn,
-			"Value type is not yet set");
-	return false;
-    }
-
-    if (type != refType)
-    {
-	compLogMessage ("core", CompLogLevelWarn,
-			"Value type does not match (is %d, expected %d)",
-			type, refType);
-	return false;
-    }
-
-    return true;
-}
-
-void
-PrivateValue::reset ()
-{
-    switch (type) {
-	case CompOption::TypeString:
-	    string = "";
-	    break;
-	case CompOption::TypeMatch:
-	    match = CompMatch ();
-	    break;
-	case CompOption::TypeAction:
-	    action = CompAction ();
-	    break;
-	case CompOption::TypeList:
-	    list.clear ();
-	    listType = CompOption::TypeBool;
-	    break;
-	default:
-	    break;
-    }
-    type = CompOption::TypeBool;
+    return boost::get< std::vector<Value> >(mValue);
+}
+
+bool
+CompOption::Value::operator== (const Value & rhs) const
+{
+    return mValue == rhs.mValue;
+}
+
+bool
+CompOption::Value::operator!= (const Value & rhs) const
+{
+    return !(mValue == rhs.mValue);
 }
 
 CompOption::Restriction::Restriction () :
@@ -664,10 +337,9 @@
 
 static void
 finiScreenOptionValue (CompScreen        *s,
-		       CompOption::Value &v,
-		       CompOption::Type  type)
+		       CompOption::Value &v)
 {
-    switch (type) {
+    switch (v.type()) {
 	case CompOption::TypeAction:
 	case CompOption::TypeKey:
 	case CompOption::TypeButton:
@@ -679,7 +351,7 @@
 
 	case CompOption::TypeList:
 	    foreach (CompOption::Value &val, v.list ())
-		finiScreenOptionValue (s, val, v.listType ());
+		finiScreenOptionValue (s, val);
 	    break;
 
 	default:
@@ -689,7 +361,7 @@
 
 CompOption::~CompOption ()
 {
-    finiOptionValue (priv->value, priv->type);
+    finiOptionValue (priv->value);
     delete priv;
 }
 

=== modified file 'src/privateoption.h'
--- src/privateoption.h	2010-11-09 14:13:19 +0000
+++ src/privateoption.h	2012-01-18 14:51:30 +0000
@@ -56,32 +56,6 @@
 	RestrictionUnion rest;
 };
 
-typedef union {
-    bool	   b;
-    int		   i;
-    float	   f;
-    unsigned short c[4];
-} ValueUnion;
-
-class PrivateValue {
-    public:
-	PrivateValue ();
-	PrivateValue (const PrivateValue&);
-
-	void reset ();
-	bool checkType (CompOption::Type refType);
-
-	CompOption::Type          type;
-	ValueUnion                value;
-	CompString                string;
-	CompAction                action;
-	CompMatch                 match;
-	CompOption::Type          listType;
-	CompOption::Value::Vector list;
-
-	bool			  active;
-};
-
 class PrivateOption
 {
     public:

=== added directory 'src/tests'
=== added file 'src/tests/CMakeLists.txt'
--- src/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/tests/CMakeLists.txt	2012-01-18 14:51:30 +0000
@@ -0,0 +1,20 @@
+include_directories( 
+  ${CMAKE_CURRENT_SOURCE_DIR} 
+)
+
+add_executable( 
+  compiz_option_test
+  
+  ${CMAKE_CURRENT_SOURCE_DIR}/option.cpp
+)
+
+target_link_libraries( 
+  compiz_option_test 
+  
+  compiz_core
+  
+  ${GTEST_BOTH_LIBRARIES}
+  ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
+)
+
+add_test( compiz_option_test compiz_option_test )

=== added file 'src/tests/option.cpp'
--- src/tests/option.cpp	1970-01-01 00:00:00 +0000
+++ src/tests/option.cpp	2012-01-18 14:51:30 +0000
@@ -0,0 +1,43 @@
+#include <gtest/gtest.h>
+
+#include "core/core.h"
+#include "core/action.h"
+#include "core/match.h"
+#include "core/option.h"
+
+namespace {
+    template<typename T>
+    void
+    check_type_value(CompOption::Type type, const T & value)
+    {
+	CompOption::Value v;
+	v.set(value);
+	ASSERT_EQ(v.type(),type);
+	ASSERT_EQ (v.get<T>(),value);
+    }
+}
+
+
+
+static unsigned short defaultColor[4] = { 0x0, 0x0, 0x0, 0xffff};
+
+TEST(CompOption,Value)
+{
+
+    check_type_value<bool> (CompOption::TypeBool, true);
+    check_type_value<bool> (CompOption::TypeBool, false);
+
+    check_type_value<int> (CompOption::TypeInt, 1);
+    check_type_value<float> (CompOption::TypeFloat, 1.f);
+    check_type_value<CompString> (CompOption::TypeString, CompString("Check"));
+
+    check_type_value<CompAction> (CompOption::TypeAction, CompAction());
+    check_type_value<CompMatch> (CompOption::TypeMatch, CompMatch());
+
+    check_type_value<CompOption::Value::Vector> (CompOption::TypeList, CompOption::Value::Vector(5));
+
+    CompOption::Value v1, v2;
+    ASSERT_EQ (v1,v2);
+    v1.set (CompString("SomeString"));
+    ASSERT_TRUE(v1 != v2);
+}