--- a/mde/lookup/Options.d	Tue Nov 25 18:01:44 2008 +0000
+++ b/mde/lookup/Options.d	Wed Nov 26 13:07:46 2008 +0000
@@ -13,19 +13,8 @@
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>. */
 
-//FIXME: Ddoc is outdated
-/** This module handles stored options, currently all except input maps.
- *
- * The purpose of having all options centrally controlled is to allow generic handling by the GUI
- * and ease saving and loading of values. The Options class is only really designed around handling
- * small numbers of variables for now.
- *
- * Note: This module uses some non-spec functionality, which "works for me", but may need to be
- * changed if it throws up problems. Specifically: templated virtual functions (Options.set, get
- * and list), and accessing private templates from an unrelated class (Options.TName, TYPES).
- * OptionChanges used to have a templated set member function (used by Options.set), which caused
- * linker problems when the module wasn't compiled from scratch.
- */
+/** This module handles loading and saving of, and allows generic access to named option variables
+ * of a simple type (see Options.TYPES). */
 module mde.lookup.Options;
 
 import mde.setup.paths;
@@ -45,50 +34,43 @@
     logger = Log.getLogger ("mde.lookup.Options");
 }
 
-//FIXME: Ddoc is outdated
-/** Base class for handling options.
-*
-* This class itself handles no options and should not be instantiated, but provides a sub-classable
-* base for generic options handling. Also, the static portion of this class tracks sub-class
-* instances and provides loading and saving methods.
-*
-* Each sub-class provides named variables for maximal-speed reading. Local sub-class references
-* should be used for reading variables, and via the addOptionsClass() hook will be loaded from
-* files during pre-init (init0 stage). Do not write changes directly to the subclasses or they will
-* not be saved; instead use set(), for example, miscOpts.set!(char[])("L10n","en-GB"). Use an
-* example like MiscOptions as a template for creating a new Options sub-class.
-*
-* Optionally, overload the validate() function. This is called after loading, allowing conditions
-* to be enforced on variables. Use set!()() to change the variables. If an exception is thrown,
-* init will abort and the executable won't start.
-*
-* Details: Options sub-classes hold associative arrays of pointers to all option variables, with a
-* char[] id. This list is used for saving, loading and to provide generic GUI options screens. The
-* built-in support in Options is only for bool, int and char[] types (a float type may get added).
-* Further to this, a generic class is used to store all options which have been changed, and if any
-* have been changed, is merged with options from the user conf dir and saved on exit.
-*/
-/* An idea for potentially extending Options, but which doesn't seem necessary now:
-Move static code from Options to an OptionSet class, which may be sub-classed. These sub-classes
-may be hooked in to the master OptionSet class to shadow all Options classes and be notified of
-changes, and may or may not have values loaded from files during init. Change-sets could be
-rewritten to use this.
-However, only the changesets should need to be notified of each change (gui interfaces could simply
-be notified that a change occured and redraw everything; users of options can just re-take their
-values every time they use them). */
+/*************************************************************************************************
+ * This class and the OptionChanges class contain all the functionality.
+ * 
+ * Options are stored in derived class instances, tracked by the static portion of Options. Each
+ * value is stored in a ValueContent class, whose value can be accessed with opCall, opCast and
+ * opAssign. These class objects can be given callbacks called whenever their value is changed.
+ * 
+ * Public static methods allow getting the list of tracked sub-class instances, and loading and saving
+ * option values. A public non-static method allows generic access to option variables.
+ * 
+ * Generic access to Options is of most use to a gui, allowing Options to be edited generically.
+ * 
+ * The easiest way to use Options is to use an existing sub-class as a template, e.g. MiscOptions.
+ *************************************************************************************************/
 class Options : IDataSection
 {
-    protected this() {}   /// Do not instantiate directly.
+    /** Do not instantiate directly; use a sub-class.
+     *
+     * CTOR adds any created instance to the list of classes tracked statically for loading/saving
+     * and generic access.
+     * 
+     * Normally instances are created by a static CTOR. */
+    protected this(char[] name)
+    in {
+	assert (((cast(ID) name) in subClasses) is null);  // Don't allow a silent replacement
+    } body {
+	subClasses[cast(ID) name] = this;
+    }
     
-    // All supported types, for generic handling via templates. It should be possible to change
-    // the supported types simply by changing this list now (untested).
-    template store(A...) { alias A store; }
-    // NOTE: currently all types have transitioned to the new method, but the old method remains
-    alias store!(bool, int, double, char[]) TYPES;    // all types
-    alias store!(bool, int, double, char[]) CTYPES;   // types stored with a content
     //BEGIN Templates: internal
-    private {
-        // Get name of a type. Basically just stringof, but special handling for arrays.
+    package {
+	// All supported types, for generic handling via templates. It should be possible to change
+	// the supported types simply by changing this list.
+	template store(A...) { alias A store; }
+	alias store!(bool, int, double, char[]) TYPES;   // types handled
+	
+	// Get name of a type. Basically just stringof, but special handling for arrays.
         // Use TName!(T) for a valid symbol name, and T.stringof for a type.
         template TName(T : T[]) {
             const char[] TName = TName!(T) ~ "A";
@@ -96,18 +78,8 @@
         template TName(T) {
             const char[] TName = T.stringof;
         }
-        
-        // Pointer lists
-        template PLists(A...) {
-            static if (A.length) {
-                static if (TIsIn!(A[0], CTYPES)) {
-                    const char[] PLists = PLists!(A[1..$]);
-                } else
-                    const char[] PLists = A[0].stringof~"*[ID] opts"~TName!(A[0])~";\n" ~ PLists!(A[1..$]);
-            } else
-                const char[] PLists = "";
-        }
-        
+    }
+    private {
         // True if type is one of A
         template TIsIn(T, A...) {
             static if (A.length) {
@@ -121,54 +93,25 @@
         
         // For addTag
         template addTagMixin(T, A...) {
-            static if (TIsIn!(T, CTYPES)) {
-                const char[] ifBlock = `if (tp == "`~T.stringof~`") {
+            const char[] ifBlock = `if (tp == "`~T.stringof~`") {
     auto p = id in opts;
     if (p) {
         auto q = cast(`~VContentN!(T)~`) (*p);
         if (q) q.assignNoCB = parseTo!(`~T.stringof~`) (dt);
     }
 }`;
-            } else
-                const char[] ifBlock = `if (tp == "`~T.stringof~`") {
-    `~T.stringof~`** p = id in opts`~TName!(T)~`;
-    if (p !is null) **p = parseTo!(`~T.stringof~`) (dt);
-}`;
             static if (A.length)
                 const char[] addTagMixin = ifBlock~` else `~addTagMixin!(A).addTagMixin;
             else
                 const char[] addTagMixin = ifBlock;
         }
-        
-        // For list
-        template listMixin(A...) {
-            static if (A.length) {
-                static if (TIsIn!(A, CTYPES))
-                    const char[] listMixin = listMixin!(A[1..$]);
-                else
-                    const char[] listMixin = `ret ~= opts`~TName!(A[0])~`.keys;` ~ listMixin!(A[1..$]);
-            } else
-                const char[] listMixin = ``;
-        }
     }
     //END Templates: internal
     
     
     //BEGIN Static
     static {
-    	/** Add an options sub-class to the list for loading and saving.
-    	 *
-    	 * Call from static this() (before Init calls load()). */
-    	void addOptionsClass (Options c, char[] i)
-    	in {    // Trap a couple of potential coding errors:
-            assert (c !is null);    // Instance must be created before calling addOptionsClass
-            assert (((cast(ID) i) in subClasses) is null);  // Don't allow a silent replacement
-        } body {
-            c.secName = i;
-            subClasses[cast(ID) i] = c;
-        }
-	
-	/** Get the hash map of Options classes. */
+	/** Get the hash map of Options classes. READ-ONLY. */
 	Options[ID] optionsClasses () {
 	    return subClasses;
 	}
@@ -239,81 +182,23 @@
     
     
     //BEGIN Non-static
-    /+ NOTE: according to spec: "Templates cannot be used to add non-static members or virtual
-    functions to classes." However, this appears to work (but linking problems did occur).
-    Alternative: use mixins. From OptionsChanges:
-        // setT (used to be a template, but:
-        // Templates cannot be used to add non-static members or virtual functions to classes. )
-        template setMixin(A...) {
-            static if (A.length) {
-                const char[] setMixin = `void set`~TName!(A[0])~` (ID id, `~A[0].stringof~` x) {
-                    `~TName!(T)~`s[id] = x;
-                }
-                ` ~ setMixin!(A[1..$]);
-            } else
-                const char[] setMixin = ``;
-        }+/
-    /+
-    /** Set option symbol of an Options sub-class to val.
-     *
-     * Due to the way options are handled generically, string IDs must be used to access the options
-     * via hash-maps, which is a little slower than direct access but necessary since the option
-     * must be changed in two separate places. */
-    /+deprecated void set(T) (char[] symbol, T val) {
-        static assert (TIsIn!(T,TYPES) && !TIsIn!(T, CTYPES), "Options.set does not support type "~T.stringof);
-        
-        changed = true;     // something got set (don't bother checking this isn't what it already was)
-        
-        try {
-            mixin (`*(opts`~TName!(T)~`[cast(ID) symbol]) = val;`);
-            mixin (`optionChanges.`~TName!(T)~`s[symbol] = val;`);
-        } catch (ArrayBoundsException) {
-            // log and ignore:
-            logger.error ("Options.set: invalid symbol");
-        }
-    }+/
-    /** Get option symbol of an Options sub-class.
-     *
-     * Using this method to read an option is not necessary, but allows for generic use.  */
-    deprecated T get(T) (char[] symbol) {
-        static assert (TIsIn!(T,TYPES), "Options does not support type "~T.stringof);
-        
-        mixin (`alias opts`~TName!(T)~` optsVars;`);
-        
-        try {
-            return *(optsVars[cast(ID) symbol]);
-        } catch (ArrayBoundsException) {
-            // log and ignore:
-            logger.error ("Options.get: invalid symbol");
-        }
-    }+/
-    
-    /** List the names of all options of a specific type. */
-    deprecated char[][] list () {
-        char[][] ret;
-        mixin (listMixin!(TYPES));
-        return ret;
-    }
-    
     /// Get all Options stored with a ValueContent.
     ValueContent[char[]] content() {
         return opts;
     }
     
-    /** Variable validate function, called when options are loaded from file. This implementation
-     * does nothing. */
-    void validate() {}
+    /** Variable validate function, called when options are loaded from file.
+     *
+     * This can be overridden to enforce limits on option variables, etc. */
+    protected void validate() {}
     
-    /** Boolean, telling whether translation strings have been loaded for the instance. */
-    bool transLoaded;
+    /** Translated name and description of the instance. mde.gui.content.Items loads these and the
+     * translation strings of all enclosed options simultaneously. */
+    char[] name, desc;
     
     protected {
-        char[] secName;         // name of this option setting; set null after translation is loaded
         OptionChanges optionChanges;	// all changes to options (for saving)
-    	
-        // The "pointer lists", e.g. char[]*[ID] optscharA;
-        mixin (PLists!(TYPES));
-        ValueContent[char[]] opts;      // generic list of option values
+        ValueContent[char[]] opts;	// generic list of option values
     }
     
     //BEGIN Mergetag loading/saving code
@@ -328,13 +213,6 @@
     
     //BEGIN Templates: impl & optionsThis
     private {
-        // Replace, e.g., bool, with BoolContent
-        template contentName(A) {
-            static if (TIsIn!(A, CTYPES)) {
-                const char[] contentName = VContentN!(A);
-            } else
-                const char[] contentName = A.stringof;
-        }
         // Return index of first comma, or halts if not found.
         template cIndex(char[] A) {
             static if (A.length == 0)
@@ -368,26 +246,6 @@
             else					// no match
                 const char[] parseT = parseT!(type, A[scIndex!(A)+1 .. $]);
         }
-        // May have a trailing comma. Assumes cIndex always returns less than A.$ .
-        template aaVars(char[] A) {
-            static if (A.length == 0)
-                const char[] aaVars = "";
-            else static if (A[0] == ' ')
-                const char[] aaVars = aaVars!(A[1..$]);
-            else
-                const char[] aaVars = "\""~A[0..cIndex!(A)]~"\"[]:&"~A[0..cIndex!(A)] ~ "," ~
-                        aaVars!(A[cIndex!(A)+1..$]);
-        }
-        // May have a trailing comma. Assumes cIndex always returns less than A.$ .
-        template aaVarsContent(char[] A) {//FIXME
-            static if (A.length == 0)
-            const char[] aaVarsContent = "";
-            else static if (A[0] == ' ')
-                const char[] aaVarsContent = aaVarsContent!(A[1..$]);
-            else
-                const char[] aaVarsContent = "\""~A[0..cIndex!(A)]~"\"[]:cast(ValueContent)"~A[0..cIndex!(A)] ~ "," ~
-                aaVarsContent!(A[cIndex!(A)+1..$]);
-        }
         // strip Trailing Comma
         template sTC(char[] A) {
             static if (A.length && A[$-1] == ',')
@@ -395,15 +253,6 @@
             else
                 const char[] sTC = A;
         }
-        // if string is empty (other than space) return null, otherwise enclose: [A]
-        template listOrNull(char[] A) {
-            static if (A.length == 0)
-                const char[] listOrNull = "null";
-            else static if (A[0] == ' ')
-                const char[] listOrNull = listOrNull!(A[1..$]);
-            else
-                const char[] listOrNull = "["~A~"]";
-        }
         // if B is empty return an empty string otherswise return what's below:
         template catOrNothing(char[] A,char[] B) {
             static if (B.length)
@@ -424,17 +273,14 @@
         // for recursing on TYPES
         template optionsThisInternal(char[] A, B...) {
             static if (B.length) {
-                static if (TIsIn!(B[0], CTYPES)) {
-                    const char[] optionsThisInternal = createContents!(B[0],parseT!(B[0].stringof,A))~
-                    optionsThisInternal!(A,B[1..$]);
-                } else
-                const char[] optionsThisInternal = `opts`~TName!(B[0])~` = `~listOrNull!(sTC!(aaVars!(parseT!(B[0].stringof,A))))~";\n" ~ optionsThisInternal!(A,B[1..$]);
-            } else
+                const char[] optionsThisInternal = createContents!(B[0],parseT!(B[0].stringof,A))~
+		    optionsThisInternal!(A,B[1..$]);
+	    } else
                 const char[] optionsThisInternal = ``;
         }
         template declValsInternal(char[] A, B...) {
             static if (B.length) {
-                const char[] declValsInternal = catOrNothing!(contentName!(B[0]),parseT!(B[0].stringof,A)) ~ declValsInternal!(A,B[1..$]);
+                const char[] declValsInternal = catOrNothing!(VContentN!(B[0]),parseT!(B[0].stringof,A)) ~ declValsInternal!(A,B[1..$]);
             } else
                 const char[] declValsInternal = ``;
         }
@@ -450,6 +296,7 @@
         template optionsThis(char[] A) {
             const char[] optionsThis =
                     "optionChanges = new OptionChanges;\n" ~
+                    "super (name);\n" ~
                     optionsThisInternal!(A,TYPES);
         }
         /+ Needs too many custom parameters to be worth it? Plus makes class less readable.
@@ -467,8 +314,8 @@
          * In case this() needs to be customized, mixin(impl!(A)) is equivalent to:
          * ---
          * mixin (declVals!(A)~`
-        this () {
-    	`~optionsThis!(A)~`
+	this (char[] name) {
+	`~optionsThis!(A)~`
 	}`);
          * ---
          *
@@ -480,14 +327,20 @@
          * Extending: mixins could also be used for the static this() {...} or even the whole
          * class, but doing so would rather decrease readability of any implementation. */
         template impl(char[] A /+, char[] symb+/) {
-            const char[] impl = declVals!(A)~"\nthis(){\n"~optionsThis!(A)~"}";
+            const char[] impl = declVals!(A)~"\nthis(char[] name){\n"~optionsThis!(A)~"}";
             // ~"\nstatic this(){\n"~optClassAdd!(symb)~"}"
         }
     }
     //END Templates: impl & optionsThis
 }
 
-/** Special class to store all locally changed options, whatever the section. */
+/*************************************************************************************************
+ * Special class to store all locally changed options.
+ * 
+ * This allows only changed options and those already stored in the user directory to be saved, so
+ * that other options can be merged in from a global directory, allowing any options not locally
+ * set to be changed globally.
+ *************************************************************************************************/
 class OptionChanges : IDataSection
 {
     //BEGIN Templates
@@ -553,23 +406,27 @@
     //END Mergetag loading/saving code
 }
 
-/* NOTE: Options sub-classes are expected to use a template to ease inserting contents and
-* hide some of the "backend" functionality. Use impl as below, or read the documentation for impl.
-*
-* Each entry should have a Translation entry with humanized names and descriptions in
-* data/L10n/ClassName.mtt
-*
-* To create a new Options sub-class, just copy, paste and adjust.
-*/
-
-/** A home for all miscellaneous options, at least for now. */
+/** A home for all miscellaneous options.
+ *
+ * Also a template for deriving Options; comments explain what does what.
+ * 
+ * Translation strings for the options are looked for in data/L10n/SectionName.mtt where
+ * this ("SectionName") names the instance. */
 MiscOptions miscOpts;
 class MiscOptions : Options {
+    /* The key step is to mixin impl.
+    The syntax is just as simple variables are declared, which is how these options used to be
+    stored. Now they're enclosed in ValueContent classes; e.g. "char[] L10n;" is replaced with
+    "TextContent L10n;". The pragma statement can be uncommented to see what code gets injected
+    (note: pragma () gets called each time the module is imported as well as when it's compiled).
+    impl creates a this() function; if you want to include your own CTOR see impl's ddoc. */
     const A = "bool exitImmediately; int maxThreads, logLevel, logOutput; double pollInterval; char[] L10n;";
     //pragma (msg, impl!(A));
     mixin (impl!(A));
     
-    void validate() {
+    // Overriding validate allows limits to be enforced on variables at load time. Currently
+    // there's no infrastructure for enforcing limits when options are set at run-time.
+    override void validate() {
         // Try to enforce sensible values, whilst being reasonably flexible:
         if (maxThreads() < 1 || maxThreads() > 64) {
             logger.warn ("maxThreads must be in the range 1-64. Defaulting to 4.");
@@ -579,8 +436,9 @@
             pollInterval = 0.01;
     }
     
+    // A static CTOR is a good place to create the instance (it must be created before init runs).
     static this() {
-        miscOpts = new MiscOptions;
-        Options.addOptionsClass (miscOpts, "MiscOptions");
+	// Adds instance to Options's tracking; the string is the section name in the config files.
+	miscOpts = new MiscOptions ("MiscOptions");
     }
 }