changeset 163:24d77c52243f

Provided sensible conversions for setting the value of one AStringContent from another, along with unittest. Updated layout and Translation unittests to run.
author Diggory Hardy <diggory.hardy@gmail.com>
date Sat, 23 May 2009 15:47:32 +0200
parents 2476790223b8
children c13bded1bed3
files codeDoc/jobs.txt codeDoc/policies.txt mde/content/AStringContent.d mde/content/Content.d mde/content/IContent.d mde/content/Translation.d mde/gui/WidgetManager.d mde/gui/widget/ParentContent.d mde/gui/widget/TextWidget.d mde/gui/widget/contentFunctions.d mde/gui/widget/layout.d mde/gui/widget/miscContent.d
diffstat 12 files changed, 199 insertions(+), 83 deletions(-) [+]
line wrap: on
line diff
--- a/codeDoc/jobs.txt	Fri May 22 19:59:22 2009 +0200
+++ b/codeDoc/jobs.txt	Sat May 23 15:47:32 2009 +0200
@@ -8,6 +8,7 @@
 
 To do (importance 0-5: 0 pointless, 1 no obvious impact now, 2 todo sometime, 3 useful, 4 important, 5 urgent):
 Also search for FIXME/NOTE/BUG/WARNING comment marks.
+3   Single-line edit: pressing return should lose keyboard focus and change value
 3   Enable dragging from more widgets: bool content, enum
 3   Content: setContent specialisations, opAssign should reject more values (particularly for BoolContent).
 3   GUI: up-clicks get passed as events and activate objects
--- a/codeDoc/policies.txt	Fri May 22 19:59:22 2009 +0200
+++ b/codeDoc/policies.txt	Sat May 23 15:47:32 2009 +0200
@@ -128,7 +128,7 @@
 	When run by an end-user (with info-level logging enabled),
 	• info messages normally occur and should be understandable to end users;
 	• warn messages may occur, and may indicate problems;
-	• error messages indicate that something is definitely wrong (even if it only has a minor effect on the program as a whole);
+	• error messages indicate that something is definitely wrong (even for user input and even if it only has a minor effect on the program as a whole);
 	• fatal messages indicate a problem preventing the program from running.
 
 Log/exception messages can be divided into two categories: those aimed at end users or modders and those only aimed at developers. A short string, either a brief English message or just a code, should be defined in the code, which can either be translated to a full message by I18nTranslation or output directly. The code string need not be a long description since it can be looked up in the code.
--- a/mde/content/AStringContent.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/content/AStringContent.d	Sat May 23 15:47:32 2009 +0200
@@ -18,9 +18,12 @@
 module mde.content.AStringContent;
 public import mde.content.Content;
 
+import Ascii = tango.text.Ascii;
+import Util = tango.text.Util;
 //FIXME: efficient conversions? Need to dup result when formatting a string anyway?
 import Int = tango.text.convert.Integer;
 import Float = tango.text.convert.Float;
+import Math = tango.math.Math;
 import derelict.sdl.keysym;
 
 import tango.util.log.Log : Log, Logger;
@@ -29,6 +32,17 @@
     logger = Log.getLogger ("mde.content.AStringContent");
 }
 
+/** Union of all content types here - for dynamic cast checking against several
+ * types. */
+union UnionContent {
+    AStringContent asc;
+    BoolContent bc;
+    StringContent sc;
+    IntContent ic;
+    DoubleContent dc;
+    EnumContent ec;
+}
+
 /** Base class for content containing a simple value editable as text.
  *
  * Derived classes should implement endEdit to convert sv and assign its value
@@ -52,16 +66,11 @@
     }
     
     /** Set the content via conversion to/from string. */
-    override bool setContent (IContent c) {
-	AStringContent asc = cast (AStringContent) c;
-	if (asc !is null) {
-	    try {
-		sv = asc.toString (0);
-		endEdit;
-	    } catch (Exception) {	// invalid conversion; just reject c
-		return false;
-	    }
-	    return true;
+    override bool set (IContent c) {
+	//AStringContent asc = cast (AStringContent) c;
+	if (c !is null) {
+	    sv = c.toString (0).dup;
+	    return endEdit;
 	}
 	return false;
     }
@@ -81,10 +90,11 @@
 		sv = sv[0..pos] ~ sv[p..$];
 	    } else {			// insert character
 		char[] tail = sv[pos..$];
+		//NOTE: reallocating each keypress isn't optimal
 		sv.length = sv.length + i.length;
 		size_t npos = pos+i.length;
 		if (tail) sv[npos..$] = tail.dup;	// cannot assign with overlapping ranges
-		    sv[pos..npos] = i;
+		sv[pos..npos] = i;
 		pos = npos;
 	    }
 	} else {			// use sym; many keys output 0
@@ -136,11 +146,19 @@
         }
     }
     
-    /** Call after editing a string; return new string (may be changed/reverted). */
-    char[] endEdit ();
+    /** Call after editing a string.
+     *
+     * Returns: true if string successfully converted to value.
+     * 
+     * Should never throw; should reset sv at least when returning false. */
+    bool endEdit ();
     
 protected:
-    char[] sv;		// string of value; updated on assignment for displaying and editing
+    /* String version of value (for toString(0) and editing).
+     * WARNING: This must point to mutable memory!
+     * (Actually this isn't usually required now, but after optimising will be.)
+     * TODO: provide a buffer, for use when editing sv. */
+    char[] sv;
     size_t pos;		// editing position; used by keyStroke
 }
 
@@ -151,14 +169,14 @@
         auto valp = symbol in changed.boolData;
         if (valp)
             v = *valp;
-        sv = v ? "true" : "false";
+        sv = (v ? "true" : "false").dup;
 	super (symbol);
     }
     
     // Assign without adding change to save changeset
     void assignNoCng (bool val) {
 	v = val;
-	sv = v ? "true" : "false";
+	sv = (v ? "true" : "false").dup;
 	if (pos > sv.length) pos = sv.length;
         endEvent;
     }
@@ -171,12 +189,24 @@
     }
     alias opCall opCast;
     
-    override char[] endEdit () {
-	v = sv && (sv[0] == 't' || sv[0] == 'T' || sv[0] == '1');
-        sv = v ? "true" : "false";
+    override bool endEdit () {
+	try {
+	    sv = Util.trim (Ascii.toLower (sv));	// NOTE: sv must be in mutable memory
+	    if (sv == "false")
+		v = 0;
+	    else if (sv == "true")
+		v = 1;
+	    else 	// throws if can't convert to int:
+		v = (Int.toLong (sv) != 0);
+	} catch (Exception e) {
+	    logger.error (e.msg);
+	    sv = (v ? "true" : "false").dup;
+	    return false;
+	}
+	sv = (v ? "true" : "false").dup;
         endEvent;
         endCng;
-	return sv;
+	return true;
     }
     
     // Add change to changeset
@@ -212,10 +242,10 @@
     }
     alias opCall opCast;
     
-    override char[] endEdit () {
+    override bool endEdit () {
 	endEvent;
         endCng;
-	return sv;
+	return true;
     }
     
     void endCng () {
@@ -238,15 +268,24 @@
 	super (symbol);
     }
     
-    // NOTE: the only point of this method is to avoid int->string->int conversions,
-    // and perhaps specialise (double->int rounding?)
-    override bool setContent (IContent c) {
-	IntContent ic = cast (IntContent) c;
-	if (ic !is null) {
-	    this = ic();
+    override bool set (IContent c) {
+	UnionContent uc;
+	uc.bc = cast (BoolContent) c;
+	if (uc.bc !is null) {
+	    this = uc.bc();
 	    return true;
 	}
-	return super.setContent (c);
+	uc.ic = cast (IntContent) c;
+	if (uc.ic !is null) {
+	    this = uc.ic();
+	    return true;
+	}
+	uc.dc = cast (DoubleContent) c;
+	if (uc.dc !is null) {
+	    this = Math.rndint (uc.dc());	// round to nearest
+	    return true;
+	}
+	return super.set (c);
     }
     
     void assignNoCng (int val) {
@@ -264,16 +303,18 @@
     }
     alias opCall opCast;
     
-    override char[] endEdit () {
+    override bool endEdit () {
 	try {
             v = Int.toInt (sv);
         } catch (Exception e) {
-            logger.warn (e.msg);
-        }
+            logger.error (e.msg);
+	    sv = Int.toString (v);
+	    return false;
+	}
         sv = Int.toString (v);
         endEvent;
         endCng;
-	return sv;
+	return true;
     }
     
     void endCng () {
@@ -284,7 +325,7 @@
     int v;
 }
 
-/** Double content. */
+/** Floating-point content. */
 class DoubleContent : AStringContent
 {
     /** Create a content with _symbol name symbol. */
@@ -296,6 +337,26 @@
 	super (symbol);
     }
     
+    override bool set (IContent c) {
+	UnionContent uc;
+	uc.bc = cast (BoolContent) c;
+	if (uc.bc !is null) {
+	    this = uc.bc();
+	    return true;
+	}
+	uc.ic = cast (IntContent) c;
+	if (uc.ic !is null) {
+	    this = uc.ic();
+	    return true;
+	}
+	uc.dc = cast (DoubleContent) c;
+	if (uc.dc !is null) {
+	    this = uc.dc();
+	    return true;
+	}
+	return super.set (c);
+    }
+    
     void assignNoCng (double val) {
 	v = val;
 	sv = Float.toString (v, 8, 4);
@@ -311,16 +372,18 @@
     }
     alias opCall opCast;
     
-    override char[] endEdit () {
+    override bool endEdit () {
         try {
             v = Float.toFloat (sv);
         } catch (Exception e) {
-            logger.warn (e.msg);
-        }
+            logger.error (e.msg);
+	    sv = Float.toString (v, 8, 4);
+	    return false;
+	}
         sv = Float.toString (v, 8, 4);
         endEvent;
         endCng;
-	return sv;
+	return true;
     }
     
     void endCng () {
@@ -349,7 +412,7 @@
             e = new EnumValueContent (this, i, symPeriod~enumSymbols[i]);
         }
         enums[v].assignFromParent (true);
-        sv = enums[v].name_;
+        sv = enums[v].name_.dup;
         // Re-set the value if a saved value is found:
         auto valp = symbol in changed.enumValData;
         if (valp)
@@ -368,7 +431,7 @@
                 return;
             }
         }
-        logger.warn ("EnumContent {} assigned invalid enumeration: {}; valid: {}", symbol, enumSym, enumSymbols);
+        logger.error ("EnumContent {} assigned invalid enumeration: {}; valid: {}", symbol, enumSym, enumSymbols);
     }
     size_t opCall () {
         //debug logger.trace ("EnumContent {} returning value: {} ({})",symbol, enumSymbols[v], v);
@@ -383,25 +446,26 @@
         enums[v]  .assignFromParent (false);
         enums[val].assignFromParent (true);
 	v = val;
-        sv = enums[v].name_;
+        sv = enums[v].name_.dup;
         if (pos > sv.length) pos = sv.length;
         endEvent;
     }
     
-    override char[] endEdit () {
+    override bool endEdit () {
 	foreach (i,e; enums)
 	    if (sv == e.name_) {
 		assignNoCng (i);
 		goto break1;
 	    }
 	
-        sv = enums[v].name_;	// sv was edited; revert
+        sv = enums[v].name_.dup;	// sv was edited; revert
         logger.error ("EnumContent "~name_~" assigned invalid value; keeping value: "~sv);
 	if (pos > sv.length) pos = sv.length;
+	return false;
 	
 	break1:
 	endCng;
-	return sv;
+	return true;
     }
     
     void endCng () {
@@ -452,10 +516,12 @@
             super.assignNoCng (val);
         }
         
-        override char[] endEdit () {
-            v = sv && (sv[0] == 't' || sv[0] == 'T' || sv[0] == '1');
-            parent.childAssign (i);
-            return sv;
+        override bool endEdit () {
+	    if (super.endEdit) {
+		parent.childAssign (i);
+		return true;	// value accepted by BoolContent, not necessarily by EnumContent
+	    }
+            return false;
         }
     
     protected:
@@ -463,3 +529,53 @@
         size_t i;
     }
 }
+
+debug (mdeUnitTest) {
+    unittest {
+	bool throws (void delegate() dg) {
+	    bool r = false;
+	    try {
+		dg();
+	    } catch (Exception e) {
+		r = true;
+	    }
+	    return r;
+	}
+	
+	StringContent sc = new StringContent ("unittest.sc");
+	IntContent ic = new IntContent ("unittest.ic");
+	BoolContent bc = new BoolContent ("unittest.bc");
+	DoubleContent dc = new DoubleContent ("unittest.dc");
+	
+	logger.info ("You should see some \"invalid literal\" errors:");
+	sc = "16";
+	ic.set = sc;
+	assert (ic() == 16);
+	sc = "five";	// fails
+	ic.set = sc;
+	assert (ic.toString(0) == "16");
+	
+	bc.set = ic;
+	assert (bc());
+	sc = "fALse";
+	bc.set = sc;
+	assert (!bc());
+	
+	sc = "31.5";
+	ic.set = sc;	// parses as int which fails
+	assert (ic() == 16);
+	dc.set = sc;
+	ic.set = dc;	// rounds to even
+	assert (ic() == 32);
+	dc = -1.5;
+	ic.set = dc;	// rounds to even
+	assert (ic() == -2);
+	
+	bc.set = dc;	// fails: not included conversion
+	assert (!bc());
+	bc.set = ic;
+	assert (bc());
+	
+	logger.info ("Unittest complete.");
+    }
+}
\ No newline at end of file
--- a/mde/content/Content.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/content/Content.d	Sat May 23 15:47:32 2009 +0200
@@ -138,7 +138,7 @@
     
     /** A naive implementation which assumes the passed content is
      * incompatible. */
-    override bool setContent (IContent) {
+    override bool set (IContent) {
 	return false;
     }
     
@@ -190,7 +190,7 @@
         list_ ~= x;
     }
     
-    override bool setContent (IContent c) {
+    override bool set (IContent c) {
 	IContentList cl = cast (IContentList) c;
 	if (cl !is null) {
 	    list_ = cl.list();
--- a/mde/content/IContent.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/content/IContent.d	Sat May 23 15:47:32 2009 +0200
@@ -46,6 +46,6 @@
      * If cont's type is compatible, the method should set its instance's
      * value to that of cont and return true, otherwise it should return false.
      */
-    bool setContent (IContent);
+    bool set (IContent);
 }
 
--- a/mde/content/Translation.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/content/Translation.d	Sat May 23 15:47:32 2009 +0200
@@ -131,7 +131,8 @@
 	}
     }
     
-    /+ Getters for entries... not wanted now.
+    // Getters for entries... not wanted now.
+debug (mdeUnitTest) {	// still used by unittest (useful for checking loading and merging)
     alias entry opCall;	    /// Convenience alias
     
     /** Get the translation for the given identifier.
@@ -168,7 +169,8 @@
             ret.name = id;
             return ret;
         }
-    } +/
+    }
+}
     
     /** This struct is used to store each translation name and description pair.
      *
--- a/mde/gui/WidgetManager.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/gui/WidgetManager.d	Sat May 23 15:47:32 2009 +0200
@@ -298,7 +298,7 @@
 	updateUnderMouse (cx, cy, state);
 	
 	// end of a drag?
-	if (dragStart !is null && b == 1 && state == false) {
+	if (dragStart !is null && b == dragButton && state == false) {
 	    if (dragStart.dragRelease (cx, cy, underMouse)) {
 		dragStart = null;
 		return;
@@ -314,7 +314,7 @@
 	}
 	
 	// Finally, post the actual event:
-	if (b == 3 && state) {	// right click - open context menu
+	if (b == 3 && !state) {	// right click - open context menu
 	    IContent contextContent = underMouse.content;
 	    if (contextContent is null) return;
 	    // NOTE: Creates new widgets every time; not optimal
@@ -328,8 +328,10 @@
 		keyFocus = underMouse;
 		setLetterCallback (&underMouse.keyEvent);
 	    }
-	    if (ret & 2)	// drag events requested
+	    if (ret & 2 && dragStart is null) {	// drag events requested
 		dragStart = underMouse;
+		dragButton = b;	// currently we allow any button to be used for a drag, but.. ?
+	    }
 	}
     }
     
@@ -503,6 +505,7 @@
     IChildWidget popupContext;		// context menu (active if not null)
     
     IChildWidget dragStart;		// if non-null, this widget should receive motion and click-release events
+    int dragButton;			// index of button in use for drag
     IChildWidget keyFocus;		// widget receiving keyboard input
     IChildWidget underMouse;		// widget under the mouse pointer
     
--- a/mde/gui/widget/ParentContent.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/gui/widget/ParentContent.d	Sat May 23 15:47:32 2009 +0200
@@ -79,7 +79,7 @@
     }
     
     override bool dropContent (IContent content) {
-	if (content_.setContent (content))
+	if (content_.set (content))
 	    return true;
 	return parent.dropContent (content);
     }
--- a/mde/gui/widget/TextWidget.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/gui/widget/TextWidget.d	Sat May 23 15:47:32 2009 +0200
@@ -150,18 +150,21 @@
     
     /** On click, request keyboard input. */
     override int clickEvent (wdabs cx, wdabs, ubyte, bool state) {
-	if (state) {
-	    //adapter.index = content_.editIndex;
+	if (state)
+	    return 2;
+	else {
 	    content_.editIndex = adapter.setIndex (cx - x);
 	    mgr.requestRedraw;
-	    return 3;	// get keyboard input via keyEvent
+	    return 1;	// get keyboard input via keyEvent
 	}
     }
     
     override bool dragRelease (wdabs, wdabs, IChildWidget widg) {
-	if (widg !is this)	// don't copy content to self
+	if (widg !is this) {	// don't copy content to self
 	    widg.dropContent (content_);
-	return true;
+	    return true;	// don't pass click as well as copy
+	}
+	return false;
     }
     
     override void keyEvent (ushort s, char[] i) {
@@ -176,13 +179,14 @@
 	mgr.requestRedraw;
     }
     override void keyFocusLost () {
-	adapter.text = content_.endEdit;	// update other users of content_ relying on callbacks
+	content_.endEdit;
+	adapter.text = content_.toString (0);
 	adapter.index;
 	mgr.requestRedraw;
     }
     
     override bool dropContent (IContent content) {
-	if (content_.setContent (content))
+	if (content_.set (content))
 	    return true;
 	return parent.dropContent (content);
     }
--- a/mde/gui/widget/contentFunctions.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/gui/widget/contentFunctions.d	Sat May 23 15:47:32 2009 +0200
@@ -51,8 +51,8 @@
     if (cast(AStringContent) c) {
         if (cast(EnumContent) c)	// can be PopupMenuWidget or ContentListWidget
             return new PopupMenuWidget(mgr,parent,id,data,c);
-        if (cast(BoolContent) c)
-            return new BoolContentWidget(mgr,parent,id,data,c);
+//         if (cast(BoolContent) c)
+//             return new BoolContentWidget(mgr,parent,id,data,c);
         return new AStringContentWidget(mgr,parent,id,data,c);
     }
     if (cast(IContentList) c)
--- a/mde/gui/widget/layout.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/gui/widget/layout.d	Sat May 23 15:47:32 2009 +0200
@@ -121,7 +121,7 @@
     }
     
     override bool dropContent (IContent content) {
-	if (cList.setContent (content))
+	if (cList.set (content))
 	    return true;
 	return parent.dropContent (content);
     }
@@ -919,23 +919,13 @@
     }
     
     debug (mdeUnitTest) unittest {
-        bool throws (void delegate() dg) {
-            bool r = false;
-            try {
-                dg();
-            } catch (Exception e) {
-                r = true;
-            }
-            return r;
-        }
-        
         AlignColumns a, a2, b;
-        a = getInstance ("a", 2);
-        a2 = getInstance ("a", 2);
-        b = getInstance ("b", 5);
+        a = getInstance ("a", null, 2, false);
+        a2 = getInstance ("a", null, 2, false);
+        b = getInstance ("a", null, 5, true);
         assert (a is a2);
         assert (a !is b);
-        assert (throws ({ getInstance ("a", 4); }));
+        assert (getInstance ("a", null, 4, false) !is a);
         
         a.setup (0, 3);
         a.spacing = 6;
--- a/mde/gui/widget/miscContent.d	Fri May 22 19:59:22 2009 +0200
+++ b/mde/gui/widget/miscContent.d	Sat May 23 15:47:32 2009 +0200
@@ -51,7 +51,7 @@
     }
     
     override bool dropContent (IContent content) {
-	if (content_.setContent (content))
+	if (content_.set (content))
 	    return true;
 	return parent.dropContent (content);
     }