utils/sexp: Clean up API and implementation

Also update the tests and command-parser.

lib/utils/mu-command-parser.cc

@@ -18,6 +18,7 @@
 */
 
 #include "mu-command-parser.hh"
+#include "mu-error.hh"
 #include "mu-utils.hh"
 
 #include <iostream>
@@ -25,27 +26,22 @@
 
 using namespace Mu;
 using namespace Command;
-using namespace Sexp;
-
-using Type = Node::Type;
-
-static Mu::Error
-command_error(const std::string& msg)
-{
-        return Mu::Error(Error::Code::Command,  msg);
-}
 
 void
-Command::invoke(const Command::CommandMap& cmap, const Node& call)
+Command::invoke(const Command::CommandMap& cmap, const Sexp& call)
 {
-        if (call.type() != Type::List || call.elements().empty() ||
+        if (!call.is_call()) {
-            call.elements().at(0).type() != Type::Symbol)
+                throw Mu::Error{Error::Code::Command,
-                throw command_error("call must be a list starting with a symbol");
+                                "expected call-sexpr but got %s",
+                                call.to_string().c_str()};
+        }
 
-        const auto& params{call.elements()};
+        const auto& params{call.list()};
         const auto cmd_it = cmap.find(params.at(0).value());
         if (cmd_it == cmap.end())
-                throw command_error("unknown command '" + params.at(0).value() + "'");
+                throw Mu::Error{Error::Code::Command,
+                                "unknown command in call %s",
+                                call.to_string().c_str()};
 
         const auto& cinfo{cmd_it->second};
 
@@ -59,8 +55,7 @@ Command::invoke(const Command::CommandMap& cmap, const Node& call)
                 // so, we're looking for the odd-numbered parameters.
                 const auto param_it = [&]() {
                         for (size_t i = 1; i < params.size(); i += 2)
-                                if (params.at(i).type() == Type::Symbol &&
+                                if (params.at(i).is_symbol() && params.at(i).value() == argname)
-                                    params.at(i).value() == argname)
                                         return params.begin() + i + 1;
 
                         return params.end();
@@ -70,24 +65,29 @@ Command::invoke(const Command::CommandMap& cmap, const Node& call)
                 // it's an error when a required parameter is missing.
                 if (param_it == params.end()) {
                         if (arginfo.required)
-                                throw command_error("missing required parameter '" + argname + "'");
+                                throw Mu::Error{Error::Code::Command,
+                                                "missing required parameter %s in call %s",
+                                                argname.c_str(), call.to_string().c_str()};
                         continue; // not required
                 }
 
                 // the types must match, but the 'nil' symbol is acceptable as
                 // "no value"
-                if (param_it->type() != arginfo.type &&
+                if (param_it->type() != arginfo.type && !(param_it->is_nil()))
-                    !(param_it->type() == Type::Symbol && param_it->value() == "nil"))
+                        throw Mu::Error{Error::Code::Command,
-                        throw command_error("parameter '" + argname + "' expects type " +
+                                        "parameter %s expects type %s, but got %s in call %s",
-                                            to_string(arginfo.type) +
+                                        argname.c_str(), to_string(arginfo.type).c_str(),
-                                            " but got " + to_string(param_it->type()));
+                                        to_string(param_it->type()).c_str(),
+                                        call.to_string().c_str()};
         }
 
         // all passed parameters must be known
         for (size_t i = 1; i < params.size(); i += 2) {
                 if (std::none_of(cinfo.args.begin(), cinfo.args.end(),
                                  [&](auto&& arg) {return params.at(i).value() == arg.first;}))
-                        throw command_error("unknown parameter '" + params.at(i).value() + "'");
+                        throw Mu::Error{Error::Code::Command,
+                                        "unknown parameter %s in call %s",
+                                        params.at(i).value().c_str(), call.to_string().c_str()};
         }
 
         if (cinfo.handler)
@@ -108,36 +108,35 @@ find_param_node (const Parameters& params, const std::string& argname)
 
         for (size_t i = 1; i < params.size(); i += 2) {
                 if (i + 1 != params.size() &&
-                    params.at(i).type() == Type::Symbol &&
+                    params.at(i).is_symbol() && params.at(i).value() == argname)
-                    params.at(i).value() == argname)
                         return params.begin() + i + 1;
         }
 
         return params.end();
 }
 
-constexpr auto Nil = "nil";
 
-static bool
+static Error
-is_nil(const Node& node)
+wrong_type (Sexp::Type expected, Sexp::Type got)
 {
-        return node.type() == Type::Symbol && node.value() == Nil;
+        return Error(Error::Code::InvalidArgument,
+                     "expected <%s> but got <%s>",
+                     to_string(expected).c_str(),
+                     to_string(got).c_str());
 }
 
+
 const std::string&
 Command::get_string_or (const Parameters& params, const std::string& argname,
                         const std::string& alt)
 {
         const auto it = find_param_node (params, argname);
-        if (it == params.end() || is_nil(*it))
+        if (it == params.end() || it->is_nil())
                 return alt;
-        else if (it->type() != Type::String)
+        else if (!it->is_string())
-                throw Error(Error::Code::InvalidArgument,
+                throw wrong_type(Sexp::Type::String, it->type());
-                            "expected <string> but got %s (value: '%s')",
+        else
-                            to_string(it->type()).c_str(),
+                return it->value();
-                            it->value().c_str());
-
-        return it->value();
 }
 
 const std::string&
@@ -145,15 +144,12 @@ Command::get_symbol_or (const Parameters& params, const std::string& argname,
                         const std::string& alt)
 {
         const auto it = find_param_node (params, argname);
-        if (it == params.end() || is_nil(*it))
+        if (it == params.end() || it->is_nil())
                 return alt;
-        else if (it->type() != Type::Symbol)
+        else if (!it->is_symbol())
-                throw Error(Error::Code::InvalidArgument,
+                throw wrong_type(Sexp::Type::Symbol, it->type());
-                            "expected <symbol> but got %s (value: '%s')",
+        else
-                            to_string(it->type()).c_str(),
+                return it->value();
-                            it->value().c_str());
-
-        return it->value();
 }
 
 
@@ -162,12 +158,10 @@ Command::get_int_or (const Parameters& params, const std::string& argname,
                      int alt)
 {
         const auto it = find_param_node (params, argname);
-        if (it == params.end() || is_nil(*it))
+        if (it == params.end() || it->is_nil())
                 return alt;
-        else if (it->type() != Type::Number)
+        else if (!it->is_number())
-                throw Error(Error::Code::InvalidArgument,
+                throw wrong_type(Sexp::Type::Number, it->type());
-                            "expected <integer> but got %s",
-                            to_string(it->type()).c_str());
         else
                 return ::atoi(it->value().c_str());
 }
@@ -179,31 +173,25 @@ Command::get_bool_or (const Parameters& params, const std::string& argname,
         const auto it = find_param_node (params, argname);
         if (it == params.end())
                 return alt;
-        else if (it->type() != Type::Symbol)
+        else if (!it->is_symbol())
-                throw Error(Error::Code::InvalidArgument,
+                throw wrong_type(Sexp::Type::Symbol, it->type());
-                            "expected <symbol> but got %s",
-                            to_string(it->type()).c_str());
         else
-                return it->value() != Nil;
+                return it->is_nil() ? false : true;
 }
 
 std::vector<std::string>
 Command::get_string_vec  (const Parameters& params, const std::string& argname)
 {
         const auto it = find_param_node (params, argname);
-        if (it == params.end() || is_nil(*it))
+        if (it == params.end() || it->is_nil())
                 return {};
-        else if (it->type() != Type::List)
+        else if (!it->is_list())
-                throw Error(Error::Code::InvalidArgument,
+                throw wrong_type(Sexp::Type::List, it->type());
-                            "expected <list> but got %s",
-                            to_string(it->type()).c_str());
 
         std::vector<std::string> vec;
-        for (const auto& n: it->elements()) {
+        for (const auto& n: it->list()) {
-                if (n.type() != Type::String)
+                if (!n.is_string())
-                        throw Error(Error::Code::InvalidArgument,
+                        throw wrong_type(Sexp::Type::String, n.type());
-                                    "expected string element but got %s",
-                                    to_string(n.type()).c_str());
                 vec.emplace_back (n.value());
         }
 

lib/utils/mu-command-parser.hh

@@ -49,10 +49,10 @@ namespace Command {
 
 /// Information about a function argument
 struct ArgInfo {
-        ArgInfo (Sexp::Node::Type typearg, bool requiredarg, std::string&& docarg):
+        ArgInfo (Sexp::Type typearg, bool requiredarg, std::string&& docarg):
                 type{typearg}, required{requiredarg},docstring{std::move(docarg)}
                 {}
-        const Sexp::Node::Type type; /**< Sexp::Node::Type of the argument */
+        const Sexp::Type type; /**< Sexp::Type of the argument */
         const bool             required; /**< Is this argument required? */
         const std::string      docstring; /**< Documentation */
 };
@@ -60,7 +60,7 @@ struct ArgInfo {
 /// The arguments for a function, which maps their names to the information.
 using ArgMap     = std::unordered_map<std::string, ArgInfo>;
 // The parameters to a Handler.
-using Parameters = Sexp::Node::Seq;
+using Parameters = Sexp::Seq;
 
 int                get_int_or    (const Parameters& parms, const std::string& argname, int alt=0);
 bool               get_bool_or   (const Parameters& parms, const std::string& argname, bool alt=false);
@@ -108,7 +108,7 @@ struct CommandInfo {
 using CommandMap = std::unordered_map<std::string, CommandInfo>;
 
 /**
- * Validate that the call (a Sexp::Node) specifies a valid call, then invoke it.
+ * Validate that the call (a Sexp) specifies a valid call, then invoke it.
  *
  * A call uses keyword arguments, e.g. something like:
  *    (foo :bar 1 :cuux "fnorb")
@@ -118,7 +118,7 @@ using CommandMap = std::unordered_map<std::string, CommandInfo>;
  * @param cmap map of commands
  * @param call node describing a call.
  */
-void invoke(const Command::CommandMap& cmap, const Sexp::Node& call);
+void invoke(const Command::CommandMap& cmap, const Sexp& call);
 
 
 static inline std::ostream&

lib/utils/mu-sexp.cc

@@ -23,9 +23,9 @@
 #include "mu-utils.hh"
 
 #include <sstream>
+#include <array>
 
 using namespace Mu;
-using namespace Sexp;
 
 __attribute__((format(printf, 2, 0))) static Mu::Error
 parsing_error(size_t pos, const char* frm, ...)
@@ -53,28 +53,28 @@ skip_whitespace (const std::string& s, size_t pos)
         return pos;
 }
 
-static Node parse (const std::string& expr, size_t& pos);
+static Sexp parse (const std::string& expr, size_t& pos);
 
-static Node
+static Sexp
 parse_list (const std::string& expr, size_t& pos)
 {
         if (expr[pos] != '(') // sanity check.
                 throw parsing_error(pos, "expected: '(' but got '%c", expr[pos]);
 
-        Node::Seq children;
+        Sexp::List list;
 
         ++pos;
         while (expr[pos] != ')' && pos != expr.size())
-                children.add(parse(expr, pos));
+                list.add(parse(expr, pos));
 
         if (expr[pos] != ')')
                 throw parsing_error(pos, "expected: ')' but got '%c'", expr[pos]);
         ++pos;
-        return Node::make_list(std::move(children));
+        return Sexp::make_list(std::move(list));
 }
 
 // parse string
-static Node
+static Sexp
 parse_string (const std::string& expr, size_t& pos)
 {
         if (expr[pos] != '"') // sanity check.
@@ -103,10 +103,10 @@ parse_string (const std::string& expr, size_t& pos)
                 throw parsing_error(pos, "unterminated string '%s'", str.c_str());
 
         ++pos;
-        return Node::make_string(std::move(str));
+        return Sexp::make_string(std::move(str));
 }
 
-static Node
+static Sexp
 parse_integer (const std::string& expr, size_t& pos)
 {
         if (!isdigit(expr[pos]) && expr[pos] != '-') // sanity check.
@@ -121,10 +121,10 @@ parse_integer (const std::string& expr, size_t& pos)
         for (; isdigit(expr[pos]); ++pos)
                 num += expr[pos];
 
-        return Node::make_number(::atoi(num.c_str()));
+        return Sexp::make_number(::atoi(num.c_str()));
 }
 
-static Node
+static Sexp
 parse_symbol (const std::string& expr, size_t& pos)
 {
         if (!isalpha(expr[pos]) && expr[pos] != ':') // sanity check.
@@ -134,11 +134,11 @@ parse_symbol (const std::string& expr, size_t& pos)
         for  (++pos; isalnum(expr[pos]) || expr[pos] == '-'; ++pos)
                 symbol += expr[pos];
 
-        return Node::make_symbol(std::move(symbol));
+        return Sexp::make_symbol(std::move(symbol));
 }
 
 
-static Node
+static Sexp
 parse (const std::string& expr, size_t& pos)
 {
         pos = skip_whitespace(expr, pos);
@@ -147,7 +147,7 @@ parse (const std::string& expr, size_t& pos)
                 throw parsing_error(pos, "expected: character '%c", expr[pos]);
 
         const auto kar = expr[pos];
-        const auto node =[&]() -> Node {
+        const auto node =[&]() -> Sexp {
                 if (kar == '(')
                         return parse_list (expr, pos);
                 else if (kar == '"')
@@ -165,8 +165,8 @@ parse (const std::string& expr, size_t& pos)
         return node;
 }
 
-Node
+Sexp
-Sexp::Node::make (const std::string& expr)
+Sexp::make_parse (const std::string& expr)
 {
         size_t pos{};
         auto node{::parse (expr, pos)};
@@ -179,7 +179,7 @@ Sexp::Node::make (const std::string& expr)
 
 
 std::string
-Sexp::Node::to_string () const
+Sexp::to_string () const
 {
         std::stringstream sstrm;
 
@@ -187,7 +187,7 @@ Sexp::Node::to_string () const
         case Type::List: {
                 sstrm << '(';
                 bool first{true};
-                for (auto&& child : elements()) {
+                for (auto&& child : list()) {
                         sstrm << (first ? "" : " ") << child.to_string();
                         first = false;
                 }
@@ -199,6 +199,7 @@ Sexp::Node::to_string () const
                 break;
         case Type::Number:
         case Type::Symbol:
+        case Type::Empty:
         default:
                 sstrm << value();
         }

lib/utils/mu-sexp.hh

@@ -21,17 +21,17 @@
 #ifndef MU_SEXP_HH__
 #define MU_SEXP_HH__
 
+#include <iterator>
 #include <string>
 #include <vector>
+#include <deque>
 #include <type_traits>
 
 #include "utils/mu-utils.hh"
 #include "utils/mu-error.hh"
 
 namespace Mu {
-namespace Sexp {
+/// Simple s-expression parser & list that parses lists () and atoms (strings
-
-/// Simple s-expression parser & builder that parses lists () and atoms (strings
 /// ("-quoted), (positive) integers ([0..9]+) and symbol starting with alpha or
 /// ':', then alphanum and '-')
 ///
@@ -39,18 +39,27 @@ namespace Sexp {
 
 
 /// Parse node
-struct Node {
+struct Sexp {
         /// Node type
-        enum struct Type { List, String, Number, Symbol };
+        enum struct Type { Empty, List, String, Number, Symbol };
+
 
         /**
-         * Make a node of out of an s-expression string.
+         * Default CTOR
+         */
+        Sexp():type_{Type::Empty}{}
+
+        /// Underlying data type for list
+        using Seq = std::deque<Sexp>;
+
+        /**
+         * Make a sexp out of an s-expression string.
          *
          * @param expr a string containing an s-expression
          *
          * @return the parsed s-expression, or throw Error.
          */
-        static Node make (const std::string& expr);
+        static Sexp make_parse (const std::string& expr);
 
         /**
          * Make a node for a string/integer/symbol/list value
@@ -59,97 +68,40 @@ struct Node {
           *
          * @return a node
          */
-        static Node make_string (std::string&& val) { return Node{Type::String, std::move(val)}; }
+        static Sexp make_string (std::string&& val)        { return Sexp{Type::String, std::move(val)}; }
-        static Node make_string (const std::string& val) { return Node{Type::String, std::string{val}}; }
+        static Sexp make_string (const std::string& val)   { return Sexp{Type::String, std::string(val)}; }
-        static Node make_number (int val) { return Node{Type::Number, format("%d", val)}; }
+        static Sexp make_number (int val)                  { return Sexp{Type::Number, format("%d", val)}; }
-        static Node make_symbol (std::string&& val) { return Node{Type::Symbol, std::move(val)}; }
+        static Sexp make_symbol (std::string&& val)        {
-
+                if (val.empty())
-
+                        throw Error(Error::Code::InvalidArgument, "symbol must be non-empty");
-        /// sequence of node objects
+                return Sexp{Type::Symbol, std::move(val)};
-        struct Seq {
+        }
-                /**
-                 * Add an item to a node-sequence.
-                 *
-                 * @param node item to add; moved/consumed.
-                 */
-                void add(Node&& node)             {nodes_.emplace_back(std::move(node));}
-                void add(Seq&& seq)               {add(make_list(std::move(seq)));}
-                void add(std::string&& s)         {add(make_string(std::move(s)));}
-                void add(int i)                   {add(make_number(i));}
-                // disambiguate.
-                void add_symbol(std::string&& s)  {add(make_symbol(std::move(s)));}
-
-                /**
-                 * Add a property to tne node sequence; i.e. a property-symbol
-                 * (starting with ':') and some node
-                 *
-                 * @param name name (must start with ":"), and some value or list
-                 * @param val
-                 */
-                template<typename T> void add_prop(std::string&& name, T&& val) {
-
-                        if (name.empty() || name[0] != ':')
-                                throw Error{Error::Code::InvalidArgument,
-                                                "property names must start with ':' ('%s')",
-                                                name.c_str()};
-
-                        add(make_symbol(std::move(name)));
-                        add(std::move(val));
-                }
-                void  add_prop(std::string&& name, const std::string& val) {
-                        add_prop(std::move(name), std::string(val));
-                }
-
-                // deliberately limited stl-like
-
-                /**
-                 * Is this an empty sequence?
-                 *
-                 * @return true or false
-                 */
-                bool empty() const {return nodes_.empty();}
-
-                /**
-                 * Get the number of elsements in the sequence
-                 *
-                 * @return number of elmement.
-                 */
-                size_t size() const {return nodes_.size();}
-
-                /**
-                 * Get begin iterator of the sequence
-                 *
-                 * @return iterator
-                 */
-                const auto begin()          const { return nodes_.begin(); }
-                /**
-                 * Get the end iterator of the sequnce
-                 *
-                 * @return an iterator
-                 */
-                const auto end()            const { return nodes_.end(); }
-
-                /**
-                 * Get a const ref to the item at idx.
-                 *
-                 * @param idx index,  must be < size()
-                 *
-                 * @return const ref to the item.
-                 */
-                const auto at(size_t idx)   const { return nodes_.at(idx);}
-
-        private:
-                std::vector<Node> nodes_;
-        };
 
         /**
-         * Make a list node from a sequence
          *
-         * @param seq a sequence of nodes
          *
-         * @return a node
+         * The value of this node; invalid for list nodes.
+         *
+         * @return
          */
-        static Node make_list   (Seq&& seq) { return Node{std::move(seq)}; }
+        const std::string& value() const {
+                if (is_list())
+                        throw Error(Error::Code::InvalidArgument, "no value for list");
+                if (is_empty())
+                        throw Error{Error::Code::InvalidArgument, "no value for empty"};
+                return value_;
+        }
+
+        /**
+         * The underlying container of this list node; only valid for lists
+         *
+         * @return
+         */
+        const Seq& list() const {
+                if (!is_list())
+                        throw Error(Error::Code::InvalidArgument, "not a list");
+                return seq_;
+        }
 
         /**
          * Convert a Sexp::Node to its string representation
@@ -165,75 +117,220 @@ struct Node {
          */
         Type type() const { return type_; }
 
+        ///
+        /// Helper struct to build mutable lists.
+        ///
+        struct List {
+                /**
+                 * Add a sexp to the list
+                 *
+                 * @param sexp a sexp
+                 * @param args rest arguments
+                 *
+                 * @return a ref to this List (for chaining)
+                 */
+                List& add () { return *this; }
+                List& add (Sexp&& sexp) { seq_.emplace_back(std::move(sexp)); return *this; }
+                template <typename... Args> List& add (Sexp&& sexp, Args... args) {
+                        seq_.emplace_back(std::move(sexp));
+                        seq_.emplace_back(std::forward<Args>(args)...);
+                        return *this;
+                }
+
+                /**
+                 * Add a property (i.e., :key sexp ) to the list
+                 *
+                 * @param name a property-name. Must start with ':', length > 1
+                 * @param sexp a sexp
+                 * @param args rest arguments
+                 *
+                 * @return a ref to this List (for chaining)
+                 */
+                List& add_prop (std::string&& name, Sexp&& sexp) {
+                        if (!is_prop_name(name))
+                                throw Error{Error::Code::InvalidArgument, "invalid property name ('%s')",
+                                                name.c_str()};
+                        seq_.emplace_back(make_symbol(std::move(name)));
+                        seq_.emplace_back(std::move(sexp));
+                        return *this;
+                }
+                template <typename... Args>
+                List& add_prop (std::string&& name, Sexp&& sexp, Args... args) {
+                        add_prop(std::move(name), std::move(sexp));
+                        add_prop(std::forward<Args>(args)...);
+                        return *this;
+                }
+
+                /**
+                 * Get the number of elements in the list
+                 *
+                 * @return number
+                 */
+                size_t size() const { return seq_.size();   }
+
+                /**
+                 * Is the list empty?
+                 *
+                 * @return true or false
+                 */
+                size_t empty() const { return seq_.empty(); }
+
+        private:
+                friend class Sexp;
+                Seq          seq_;
+        };
+
+        /**
+         * Construct a list sexp from a List
+         *
+         * @param list a list-list
+         * @param sexp  a Sexp
+         * @param args rest arguments
+         *
+         * @return a sexp.
+         */
+        static Sexp make_list(List&& list) {
+                return Sexp{Type::List, std::move(list.seq_)};
+        }
+        template <typename ... Args>
+        static Sexp make_list(Sexp&& sexp, Args... args) {
+                List lst;
+                lst.add(std::move(sexp)).add(std::forward<Args>(args)...);
+                return make_list(std::move(lst));
+        }
+
+        /**
+         * Construct a properrty list sexp from a List
+         *
+         * @param name the property name; must start wtth ':'
+         * @param sexp  a Sexp
+         * @param args rest arguments (property list)
+         *
+         * @return a sexp.
+         */
+        template <typename ... Args>
+        static Sexp make_prop_list(std::string&& name, Sexp&& sexp, Args... args) {
+                List list;
+                list.add_prop(std::move(name), std::move(sexp), std::forward<Args>(args)...);
+                return make_list(std::move(list));
+
+        }
+
+        /**
+         * Construct a properrty list sexp from a List
+         *
+         * @param funcname function name for the call
+         * @param name the property name; must start wtth ':'
+         * @param sexp  a Sexp
+         * @param args rest arguments (property list)
+         *
+         * @return a sexp.
+         */
+        template <typename ... Args>
+        static Sexp make_call(std::string&& funcname, std::string&& name, Sexp&& sexp, Args... args) {
+                List list;
+                list.add(make_symbol(std::move(funcname)));
+                list.add_prop(std::move(name), std::move(sexp), std::forward<Args>(args)...);
+                return make_list(std::move(list));
+
+        }
+
+
         /// Some type helpers
-        bool is_list()   const { return type() == Type::List; };
+        bool is_list()    const { return type() == Type::List;   }
-        bool is_string() const { return type() == Type::String; }
+        bool is_string()  const { return type() == Type::String; }
-        bool is_number() const { return type() == Type::Number; }
+        bool is_number()  const { return type() == Type::Number; }
-        bool is_symbol() const { return type() == Type::Symbol; }
+        bool is_symbol()  const { return type() == Type::Symbol; }
-        bool is_nil()    const { return is_symbol() && value() == "nil"; }
+        bool is_empty()   const { return type() == Type::Empty;  }
-        bool is_t()      const { return is_symbol() && value() == "t"; }
+
+        operator bool() const { return !is_empty(); }
+
+        static constexpr auto SymbolNil{"nil"};
+        static constexpr auto SymbolT{"t"};
+        bool is_nil()     const { return is_symbol() && value() == SymbolNil; }
+        bool is_t()       const { return is_symbol() && value() == SymbolT; }
 
         /**
-         * The elements of this node; invalid unless this is a list node.
+         * Is this a prop-list? A prop list is a list sexp with alternating
+         * property / sexp
          *
          * @return
          */
-        const Seq& elements() const {
+        bool is_prop_list() const {
-                if (type() != Type::List)
+                if (!is_list() || list().size() % 2 != 0)
-                        throw Error(Error::Code::InvalidArgument, "no elements for non-list");
+                        return false;
-                return seq_;
+                else
+                        return is_prop_list(list().begin(), list().end());
         }
 
         /**
-         * The value of this node; invalid for list nodes.
+         * Is this a call? A call is a list sexp with a symbol (function name),
+         * followed by a prop list
          *
          * @return
          */
-        const std::string& value() const {
+        bool is_call() const {
-                if (type_ == Type::List)
+                if (!is_list() || list().size() % 2 != 1 || !list().at(0).is_symbol())
-                        throw Error(Error::Code::InvalidArgument, "no value for list");
+                        return false;
-                return value_;
+                else
-        }
+                        return is_prop_list (list().begin()+1, list().end());
+        };
 
 private:
-        /**
+        Sexp(Type typearg, std::string&& valuearg): type_{typearg}, value_{std::move(valuearg)} {
-         * Construct a new non-list node
+                if (is_list())
+                        throw Error{Error::Code::InvalidArgument, "cannot be a list type"};
+                if (is_empty())
+                        throw Error{Error::Code::InvalidArgument, "cannot be an empty type"};
+
+        }
+        Sexp(Type typearg, Seq&& seq): type_{Type::List}, seq_{std::move(seq)}{
+                if (!is_list())
+                        throw Error{Error::Code::InvalidArgument, "must be a list type"};
+                if (is_empty())
+                        throw Error{Error::Code::InvalidArgument, "cannot be an empty type"};
+        }
+                /**
+         * Is the sexp a valid property name?
          *
-         * @param typearg the type of node
+         * @param sexp a Sexp.
-         * @param valuearg the value
+         *
+         * @return true or false.
          */
-        Node(Type typearg, std::string&& valuearg):
+        static bool is_prop_name(const std::string& str) {
-                type_{typearg}, value_{std::move(valuearg)} {
+                return str.size() > 1 && str.at(0) == ':';
-                if (typearg == Type::List)
+        }
-                        throw Error(Error::Code::Parsing,
+        static bool is_prop_name(const Sexp& sexp) {
-                                    "atomic type cannot be a <list>");
+                return sexp.is_symbol() && is_prop_name(sexp.value());
         }
 
-        /**
+        static bool is_prop_list (Seq::const_iterator b, Seq::const_iterator e) {
-         * Construct a list node
+                while (b != e) {
-         *
+                        if (!is_prop_name(*b))
-         * @param kids the list's children
+                                return false;
-         */
+                        if (++b == e)
-        explicit Node(Seq&& seq):
+                                return false;
-                type_{Type::List}, seq_{std::move(seq)}
+                        ++b;
-                {}
+                }
+                return b == e;
+        }
 
-
+        const Type        type_;   /**<  Type of node */
-        const Type        type_; /**<  Type of node */
+        const std::string value_;  /**< String value of node (only for
-        const std::string value_;    /**< String value of node (only for
+                                    * non-Type::Lst)*/
-                                     * non-Type::Lst)*/
+        const Seq         seq_;    /**< Children of node (only for
-        const Seq         seq_;  /**< Children of node (only for
+                                    * Type::Lst) */
-                                     * Type::Lst) */
 };
 
 static inline std::ostream&
-operator<<(std::ostream& os, Sexp::Node::Type id)
+operator<<(std::ostream& os, Sexp::Type id)
 {
         switch (id) {
-        case Sexp::Node::Type::List:    os << "<list>"; break;
+        case Sexp::Type::List:    os << "list"; break;
-        case Sexp::Node::Type::String:  os << "<string>"; break;
+        case Sexp::Type::String:  os << "string"; break;
-        case Sexp::Node::Type::Number:  os << "<number>"; break;
+        case Sexp::Type::Number:  os << "number"; break;
-        case Sexp::Node::Type::Symbol:  os << "<symbol>"; break;
+        case Sexp::Type::Symbol:  os << "symbol"; break;
+        case Sexp::Type::Empty:   os << "empty"; break;
         default: throw std::runtime_error ("unknown node type");
         }
 
@@ -241,15 +338,12 @@ operator<<(std::ostream& os, Sexp::Node::Type id)
 }
 
 static inline std::ostream&
-operator<<(std::ostream& os, const Sexp::Node& node)
+operator<<(std::ostream& os, const Sexp& sexp)
 {
-        os << node.to_string();
+        os << sexp.to_string();
         return os;
 }
 
-} // Sexp
-
-
 } // Mu
 
 #endif /* MU_SEXP_HH__ */

lib/utils/test-command-parser.cc

@@ -31,26 +31,27 @@ using namespace Mu;
 static void
 test_param_getters()
 {
-        const auto node { Sexp::Node::make(R"((foo :bar 123 :cuux "456" :boo nil :bah true))")};
+        const auto sexp { Sexp::make_parse(R"((foo :bar 123 :cuux "456" :boo nil :bah true))")};
 
-        std::cout << node << "\n";
+        if (g_test_verbose())
+                std::cout << sexp << "\n";
 
-        g_assert_cmpint(Command::get_int_or(node.elements(), ":bar"), ==, 123);
+        g_assert_cmpint(Command::get_int_or(sexp.list(), ":bar"), ==, 123);
-        assert_equal(Command::get_string_or(node.elements(), ":bra", "bla"), "bla");
+        assert_equal(Command::get_string_or(sexp.list(), ":bra", "bla"), "bla");
-        assert_equal(Command::get_string_or(node.elements(), ":cuux"), "456");
+        assert_equal(Command::get_string_or(sexp.list(), ":cuux"), "456");
 
-        g_assert_true(Command::get_bool_or(node.elements(),":boo") == false);
+        g_assert_true(Command::get_bool_or(sexp.list(),":boo") == false);
-        g_assert_true(Command::get_bool_or(node.elements(),":bah") == true);
+        g_assert_true(Command::get_bool_or(sexp.list(),":bah") == true);
 }
 
 
 static bool
-call (const Command::CommandMap& cmap, const std::string& sexp) try
+call (const Command::CommandMap& cmap, const std::string& str) try
 {
-        const auto node{Sexp::Node::make(sexp)};
+        const auto sexp{Sexp::make_parse(str)};
-        g_message ("invoking %s", to_string(node).c_str());
+        g_message ("invoking %s", to_string(sexp).c_str());
 
-        invoke (cmap, node);
+        invoke (cmap, sexp);
 
         return true;
 
@@ -69,8 +70,8 @@ test_command()
 
         cmap.emplace("my-command",
                      CommandInfo{
-                             ArgMap{ {":param1", ArgInfo{Sexp::Node::Type::String, true, "some string" }},
+                             ArgMap{ {":param1", ArgInfo{Sexp::Type::String, true, "some string" }},
-                                     {":param2", ArgInfo{Sexp::Node::Type::Number, false, "some integer"}}},
+                                     {":param2", ArgInfo{Sexp::Type::Number, false, "some integer"}}},
                             "My command,",
                             {}});
 
@@ -92,8 +93,8 @@ test_command2()
         cmap.emplace("bla",
                    CommandInfo{
                            ArgMap{
-                                   {":foo",  ArgInfo{Sexp::Node::Type::Number, false, "foo"}},
+                                   {":foo",  ArgInfo{Sexp::Type::Number, false, "foo"}},
-                                   {":bar",  ArgInfo{Sexp::Node::Type::String, false,  "bar"}},
+                                   {":bar",  ArgInfo{Sexp::Type::String, false,  "bar"}},
                            },"yeah",
                            [&](const auto& params){}});
 
@@ -114,8 +115,8 @@ test_command_fail()
 
         cmap.emplace("my-command",
                      CommandInfo{
-                             ArgMap{ {":param1", ArgInfo{Sexp::Node::Type::String, true, "some string" }},
+                             ArgMap{ {":param1", ArgInfo{Sexp::Type::String, true, "some string" }},
-                                     {":param2", ArgInfo{Sexp::Node::Type::Number, false, "some integer"}}},
+                                     {":param2", ArgInfo{Sexp::Type::Number, false, "some integer"}}},
                             "My command,",
                             {}});
 

lib/utils/test-sexp.cc

@@ -27,14 +27,13 @@
 #include "mu-utils.hh"
 
 using namespace Mu;
-using namespace Sexp;
 
 static bool
 check_parse (const std::string& expr, const std::string& expected)
 {
         try {
-                const auto parsed{to_string(Node::make(expr))};
+                const auto parsed{to_string(Sexp::make_parse(expr))};
-                g_assert_cmpstr(parsed.c_str(), ==, expected.c_str());
+                assert_equal(parsed, expected);
                 return true;
 
         } catch (const Error& err) {
@@ -59,54 +58,71 @@ bar")",  "\"foo\nbar\"");
 }
 
 static void
-test_builder()
+test_list()
 {
-        const auto nstr{Node::make_string("foo")};
+        const auto nstr{Sexp::make_string("foo")};
         g_assert_true(nstr.value() == "foo");
-        g_assert_true(nstr.type()  == Node::Type::String);
+        g_assert_true(nstr.type()  == Sexp::Type::String);
         assert_equal(nstr.to_string(), "\"foo\"");
 
-        const auto nnum{Node::make_number(123)};
+        const auto nnum{Sexp::make_number(123)};
         g_assert_true(nnum.value() == "123");
-        g_assert_true(nnum.type()  == Node::Type::Number);
+        g_assert_true(nnum.type()  == Sexp::Type::Number);
         assert_equal(nnum.to_string(), "123");
 
-        const auto nsym{Node::make_symbol("blub")};
+        const auto nsym{Sexp::make_symbol("blub")};
         g_assert_true(nsym.value() == "blub");
-        g_assert_true(nsym.type()  == Node::Type::Symbol);
+        g_assert_true(nsym.type()  == Sexp::Type::Symbol);
         assert_equal(nsym.to_string(), "blub");
 
-        Node::Seq seq;
+        Sexp::List list;
-        seq.add(Node::make_string("foo"));
+        list .add(Sexp::make_string("foo"))
-        seq.add(Node::make_number(123));
+                .add(Sexp::make_number(123))
-        seq.add(Node::make_symbol("blub"));
+                .add(Sexp::make_symbol("blub"));
 
-        const auto nlst = Node::make_list(std::move(seq));
+        const auto nlst = Sexp::make_list(std::move(list));
-        g_assert_true(nlst.elements().size() == 3);
+        g_assert_true(nlst.list().size() == 3);
-        g_assert_true(nlst.type() == Node::Type::List);
+        g_assert_true(nlst.type() == Sexp::Type::List);
-        g_assert_true(nlst.elements().at(1).value() == "123");
+        g_assert_true(nlst.list().at(1).value() == "123");
 
         assert_equal(nlst.to_string(),"(\"foo\" 123 blub)");
 }
 
+
+static void
+test_prop_list()
+{
+        Sexp::List l1;
+        l1.add_prop(":foo", Sexp::make_string("bar"));
+        Sexp s2{Sexp::make_list(std::move(l1))};
+        assert_equal(s2.to_string(), "(:foo \"bar\")");
+
+
+        Sexp::List l2;
+        const std::string x{"bar"};
+        l2.add_prop(":foo", Sexp::make_string(x));
+        l2.add_prop(":bar", Sexp::make_number(77));
+        Sexp::List l3;
+        l3.add_prop(":cuux", Sexp::make_list(std::move(l2)));
+        Sexp s3{Sexp::make_list(std::move(l3))};
+        assert_equal(s3.to_string(), "(:cuux (:foo \"bar\" :bar 77))");
+}
+
 static void
 test_props()
 {
-        Node::Seq seq;
+        auto sexp2 = Sexp::make_list(
+                Sexp::make_string("foo"),
+                Sexp::make_number(123),
+                Sexp::make_symbol("blub"));
 
-        seq.add_prop(":foo", "bär");
+        auto sexp = Sexp::make_prop_list(
-        seq.add_prop(":cuux", 123);
+                ":foo",  Sexp::make_string("bär"),
-        seq.add_prop(":flub", Node::make_symbol("fnord"));
+                ":cuux", Sexp::make_number(123),
+                ":flub", Sexp::make_symbol("fnord"),
+                ":boo",  std::move(sexp2));
 
-        Node::Seq seq2;
+        assert_equal(sexp.to_string(),
-        seq2.add(Node::make_string("foo"));
-        seq2.add(Node::make_number(123));
-        seq2.add(Node::make_symbol("blub"));
-
-        seq.add_prop(":boo", std::move(seq2));
-
-        Node expr = Node::make_list(std::move(seq));
-        assert_equal(expr.to_string(),
                      "(:foo \"b\303\244r\" :cuux 123 :flub fnord :boo (\"foo\" 123 blub))");
 }
 
@@ -116,13 +132,14 @@ main (int argc, char *argv[]) try
         g_test_init (&argc, &argv, NULL);
 
         if (argc == 2) {
-                std::cout << Sexp::Node::make(argv[1]) << '\n';
+                std::cout << Sexp::make_parse(argv[1]) << '\n';
                 return 0;
         }
 
-        g_test_add_func ("/utils/sexp/parser",  test_parser);
+        g_test_add_func ("/utils/sexp/parser",    test_parser);
-        g_test_add_func ("/utils/sexp/builder", test_builder);
+        g_test_add_func ("/utils/sexp/list",      test_list);
-        g_test_add_func ("/utils/sexp/props",   test_props);
+        g_test_add_func ("/utils/sexp/proplist",  test_prop_list);
+        g_test_add_func ("/utils/sexp/props",     test_props);
 
 	return g_test_run ();