Closed GoogleCodeExporter closed 9 years ago
For storing the actual data found, the best thing I've come up with so far is a
class called "Node". This has two data members:
map<string, Node> childList;
string textValue;
...both of which are private. After any "set" method is called, an
"assertValid()" function is called, and throws an exception if "childList" or
"textValue" are ever both non-empty. (Note that "textValue" will likely be a
stack<string> in the final release).
We provide a constructor that takes a "string" and also a "char*", and we have
"operator[]" return the child at that location (or throw an exception if none
exists). This allows us to do use nice, simple syntax like:
Node root;
Node child = "hello"; //Initialize now
Node child2;
root.addChild("key2", child);
root.addChild("key", child2);
root["key"] = "value"; //Initialize later
cout <<root["key"].getString() <<" " <<root["key2"].getString() <<endl;
Using "addChild" enforces that we don't accidentally build invalid nodes into
the tree. We also have an "isLeaf()" function to allow easy iterating. Leaf
nodes must always have non-empty strings.
It's not a perfect solution, but it allows us to keep Node non-virtual, and
keep Node relatively simplistic.
Original comment by seth.h...@gmail.com
on 26 Jan 2011 at 12:24
Attachments:
A tree of Nodes is now built, and the "local" options are extracted easily
enough using a lambda callback function.
I am considering a breaking change: expand "${currdir}" to the current folder,
so that:
"${currdir}\keymagic.kms" works as expected.
There are other options; for example, in :
"@keymagic.kms", the @ means "it's a file".
The problem is thus:
1) Files are always considered in the current config directory (more or less). We really don't want users loading random files.
2) Prepending the current directory as a prefix is a _semantic_ decision; we only do it to files.
3) Building a parse tree is _syntax_ only. So, we don't know if property "x" is going to require the directory prepended until long after the tree is built.
4) Unfortunately, after the tree is built, the directory information is no longer available, because each _string_ in our stack may have been loaded from a different directory.
This is actually a generalization of the problem of types in Javascript. For
example:
"true" might be a boolean value, or it might be an identifier (language named "True?")
We could just use true (JSON boolean), but that (1) won't solve our "file" type problem and (2) will confuse users to no end.
I'll probably parse booleans later anyway (it makes sense to), but it makes
sense to cheat a bit and process file names a tiny bit now. For example:
1) Prepend the full directory to the filename. (Fixes DLL __cwd() hack and the problem of an Input Method being re-defined later.)
2) Run some regexes, convert "/" to "\" (required for DLLs, doesn't hurt other files), remove two "\"'s in a row, etc.
3) Enforce that a config file can ONLY load files inside its own directory.
The question is... how to identify files! And how to crash gracefully if the
user fails to specify a file correctly?
Original comment by seth.h...@gmail.com
on 27 Jan 2011 at 10:58
Fixed the directories problem by making use of context offered later in the
build process:
1) If a file contains "." and then 1 to 4 letters at the end of the string, it's prepended with the complete directory.
2) If, later, we are expecting a raw string value (and not a directory), then search for a "\" and remove everything up to and including that.
Thus, directories are handled transparently and easily.
Original comment by seth.h...@gmail.com
on 27 Jan 2011 at 1:12
The next step is to convert our Node tree to a "OptionSet" tree. (Hopefully we
can merge "resolvePartialSettings" into something more intelligent here.)
I think we should make another tree of nodes, which compare and validate the
Node tree. Let this class be Node 2, with:
- map <string, Node2> children; //The string is what they "match" to; might need to be a regex.
- function<> do
- type = NODE | STRING | BOOL | IDENTIFIER | INT
- isLeaf() -> { return type!=NODE; }
Then, we proceed by matching children from the root node, and calling do(),
which performs things like setting the actual value or creating, e.g., a new
"Language" with the given id.
The benefits to this are:
1) We don't need to check if we have "enough" identifiers. Just do the following:
//Given Node n and Node2 n2;
if (n.isEmpty() && n!=this->root) { throw "Empty node."; }
if (n.isLeaf() != n2.isLeaf()) { throw "Type mismatch"; }
2) Likewise, we can simplify exception handling. Consider:
if (children.count(nextToken)==0) { throw "Unknown setting."; }
//Later: catch this as "ex" and:
msg <<"Error loading property: " <<n.getAbsKey() <<"=" <<n.str() <<" : " <<ex
throw msg.str();
//This allows us to force "name=value" onto every load error that occurs, with no extra work.
3) We can now catch subtle errors that weren't caught before, like "settings.hotkey.fakesuffix"="test". We can give more reliable error messages for existing errors like "languages"="myanmar".
Some difficulties:
1) How to flag errors such as "can't define a new Language in config-local", or "can't define extensions AT ALL outside of Common".
2) The actual function semantics for "type" is confusing, because we want to pass, e.g., an actual boolean as "val" for type==BOOL. I foresee some template hacking.
3) Still not sure how to avoid "partialSettings". Might have to tidy up the current "OptionTree" class. Possibly make "OptionTree2" for now, to avoid overwriting the actual option tree?
Original comment by seth.h...@gmail.com
on 28 Jan 2011 at 7:28
Added a whole bunch of settings, and a basic "permissions" system.
Currently, the node's creation sets this value, and for non-leaf nodes that's
the end of the story. We might want to update this (or even keep a separate
stack) for each call to str(), since these must be validated too ---or at least
the top ones should.
The other option, which is kind of intriguing, is to transform each file
individually, and contribute to a "partial" tree as we go. Since IDs are part
of the key (not the value), we are guaranteed to have the only "required" item
for each element, and then we don't have to save the permissions. (But, argh,
then we have to re-undo all the const CfgPerm& items we just removed...)
Adding this comment just so that I don't forget to debug permissions heavily.
Original comment by seth.h...@gmail.com
on 31 Jan 2011 at 7:24
Re-worked with the second option (transform as you go).
Now, however, we need to keep some notion of a "dirty" or "clean" node. After
reading a node's value, it should be marked "clean", and setting it again (in
the "build" phase) should mark it as "dirty". Clean nodes should not be read
again on "walk".
Theoretically, marking a node dirty should mark all parents dirty, so we can
avoid walking entire branches of the tree at a time, but this is not strictly
necessary. It might be easy, though; we could do it.
Original comment by seth.h...@gmail.com
on 31 Jan 2011 at 8:22
"dirty" and "clean" seem to work fine. I'd like to do a full trace later (with
local/user overrides and the works) just to confirm that this is indeed working
correctly.
Original comment by seth.h...@gmail.com
on 31 Jan 2011 at 8:50
Fun trivia point: did you know that "transformations" was misspelled as
"tranformations" in Wait Zar 1.8. :P
Original comment by seth.h...@gmail.com
on 31 Jan 2011 at 9:09
All settings load, and they are tracked with a new config flag: "C".
Need to write a few dummy override config files and check the following things:
1) Permisssions.
2) "Change" and "Add" differences (i.e., change a language in the local conf)
3) Dirty/clean nodes. Double-check the full tree walk. Also check for overrides.
We also need to check for things like missing settings (e.g., a language
without an encoding) and we need to do the "lastused" setting. It makes sense
to replace the stack-of-strings with just a string, because if someone defines:
languages.myanmar.default-input-method = waitzar
languages.myanmar.default-input-method = lastused
in the same _file_, then this should be silently overwritten (or maybe even
fail with an error).
We also have to log the actual item which is set while walking the tree.
Hmm.... not sure what to do first. :D
Original comment by seth.h...@gmail.com
on 31 Jan 2011 at 11:34
All the important stuff is done.
* Properties were (mostly) elevated out of pointer classes.
* Lastused doesn't actually work, but that's a minor point.
* The code for saving local settings is already halfway in MainFile; we should take the remaining code out of RuntimeConfig and put it there.
* Need to meticulously scour ConfigManager for old "test case" code, put it into the new model, and then clear out all the commented code.
* ConfigManager's "validate" method needs to be refactored out into MainFile or... something.
Original comment by seth.h...@gmail.com
on 2 Feb 2011 at 3:07
All code has been cleaned up, and generally tested.
More tests will be necessary closer to the next release, but the work completed
more than closes this bug.
Original comment by seth.h...@gmail.com
on 4 Feb 2011 at 9:36
Original issue reported on code.google.com by
seth.h...@gmail.com
on 25 Jan 2011 at 8:03