# HG changeset patch # User Diggory Hardy # Date 1243086452 -7200 # Node ID 24d77c52243f15c24c45a517debf762b30e40b03 # Parent 2476790223b82998a9e053b9d2db4c7404df7f97 Provided sensible conversions for setting the value of one AStringContent from another, along with unittest. Updated layout and Translation unittests to run. diff -r 2476790223b8 -r 24d77c52243f codeDoc/jobs.txt --- 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 diff -r 2476790223b8 -r 24d77c52243f codeDoc/policies.txt --- 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. diff -r 2476790223b8 -r 24d77c52243f mde/content/AStringContent.d --- 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 diff -r 2476790223b8 -r 24d77c52243f mde/content/Content.d --- 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(); diff -r 2476790223b8 -r 24d77c52243f mde/content/IContent.d --- 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); } diff -r 2476790223b8 -r 24d77c52243f mde/content/Translation.d --- 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. * diff -r 2476790223b8 -r 24d77c52243f mde/gui/WidgetManager.d --- 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 diff -r 2476790223b8 -r 24d77c52243f mde/gui/widget/ParentContent.d --- 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); } diff -r 2476790223b8 -r 24d77c52243f mde/gui/widget/TextWidget.d --- 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); } diff -r 2476790223b8 -r 24d77c52243f mde/gui/widget/contentFunctions.d --- 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) diff -r 2476790223b8 -r 24d77c52243f mde/gui/widget/layout.d --- 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; diff -r 2476790223b8 -r 24d77c52243f mde/gui/widget/miscContent.d --- 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); }