From c6dce94e537f394cb3126d0883de7dcb4a42ea01 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Mon, 21 May 2018 13:59:57 +0200
Subject: Implement config-list command, adjust config-add/create diagnostics

---
 bdep/config.cli  | 27 +++++++-----------
 bdep/config.cxx  | 87 +++++++++++++++++++++++++++++++++++++++++++++++---------
 bdep/project.cxx | 10 +++----
 tests/init.test  |  8 +++---
 tests/new.test   |  4 +--
 5 files changed, 95 insertions(+), 41 deletions(-)

diff --git a/bdep/config.cli b/bdep/config.cli
index 20f2a1f..8c355db 100644
--- a/bdep/config.cli
+++ b/bdep/config.cli
@@ -16,27 +16,11 @@ namespace bdep
      <prj-spec> <prj-dir>
      <cfg-args> <option> <module> <cfg-var>",
 
-    // Other potential subcommands:
-    //
-    // list        - list associated configurations
-    // show|status - details about a configuration (init'ed packages, etc)
-    //
-    // @@ Should we be able to remove config with init'ed packages? Who
-    //    is going to drop them in configs? Or require them to be de-init'ed?
-    //    A: yes, only empty configs should be removed, if not empty, suggest
-    //    to deinitialize with bdep-deinit.
-    //
-    // @@ Should set be able to alter forward flag? This will require both
-    //    disfiguring forward on old and configuring on new. Perhaps best
-    //    done by sync? I.e., for this change to take effect, one has to
-    //    run sync on both configs? Then sync would somehow need to know
-    //    to remove forwards. Maybe it should always check for stray
-    //    forwards (via out-root.build).
-    //
     "\h|SYNOPSIS|
 
      \c{\b{bdep config add} \ \ \ [<options>] [<prj-spec>] [\b{@}<cfg-name>] <cfg-dir>\n
         \b{bdep config create} [<options>] [<prj-spec>] [\b{@}<cfg-name>] <cfg-dir> [<cfg-args>]\n
+        \b{bdep config list} \ \ [<options>] [<prj-spec>] [<cfg-spec>...]\n
         \b{bdep config remove} [<options>] [<prj-spec>] <cfg-spec>... | \b{--all}|\b{-a}\n
         \b{bdep config rename} [<options>] [<prj-spec>] <cfg-spec> <cfg-name>\n
         \b{bdep config set} \ \ \ [<options>] [<prj-spec>] <cfg-spec>\n
@@ -107,6 +91,14 @@ namespace bdep
          configuration, then they must have a consistent auto-synchronization
          setting.|
 
+     \li|\cb{list}
+
+         The \cb{list} subcommand prints the list of build configurations
+         associated with the project. Unless one or more configurations are
+         specified explicitly, \cb{list} prints all the associate
+         configurations. Note that the output is written to \cb{STDOUT}, not
+         \cb{STDERR}.|
+
      \li|\cb{remove}
 
          The \cb{remove} subcommand removes one or more build configurations
@@ -132,6 +124,7 @@ namespace bdep
 
     bool add;
     bool create;
+    bool list;
     bool remove;
     bool rename;
     bool set;
diff --git a/bdep/config.cxx b/bdep/config.cxx
index 8193140..84aa77d 100644
--- a/bdep/config.cxx
+++ b/bdep/config.cxx
@@ -4,6 +4,8 @@
 
 #include <bdep/config.hxx>
 
+#include <iostream> // cout
+
 #include <bdep/database.hxx>
 #include <bdep/project-odb.hxx>
 #include <bdep/diagnostics.hxx>
@@ -12,6 +14,19 @@ using namespace std;
 
 namespace bdep
 {
+  template <typename O>
+  void
+  print_configuration (O& o, const shared_ptr<configuration>& c)
+  {
+    char s (' ');
+
+    if (c->name)       o << '@' << *c->name << ' ';
+    /*              */ o << c->path << ' ' << *c->id;
+    if (c->default_)  {o << s << "default";           s = ',';}
+    if (c->forward)   {o << s << "forwarded";         s = ',';}
+    if (c->auto_sync) {o << s << "auto-synchronized"; s = ',';}
+  }
+
   // Translate the configuration directory that is actually a name (@foo or
   // -@foo) to the real directory (prj-foo) and name (@foo).
   //
@@ -145,13 +160,8 @@ namespace bdep
     if (verb)
     {
       diag_record dr (text);
-      /*              */ dr << what << " configuration ";
-      if (r->name)       dr << '@' << *r->name << ' ';
-      /*              */ dr << r->path << " (" << *r->id;
-      if (r->default_)   dr << ", default";
-      if (r->forward)    dr << ", forwarded";
-      if (r->auto_sync)  dr << ", auto-synchronized";
-      /*              */ dr << ')';
+      dr << what << " configuration ";
+      print_configuration (dr, r);
     }
 
     return r;
@@ -287,6 +297,57 @@ namespace bdep
   }
 
   static int
+  cmd_config_list (const cmd_config_options& o, cli::scanner&)
+  {
+    tracer trace ("config_list");
+
+    dir_path prj (find_project (o));
+    database db (open (prj, trace));
+
+    transaction t (db.begin ());
+
+    configurations cfgs;
+    if (o.config_specified ()    || // Note: handling --all|-a ourselves.
+        o.config_id_specified () ||
+        o.config_name_specified ())
+    {
+      cfgs = find_configurations (o,
+                                  prj,
+                                  t,
+                                  false /* fallback_default */,
+                                  false /* validate         */);
+    }
+    else
+    {
+      using query = bdep::query<configuration>;
+
+      // We want to show the default configuration first, then sort them
+      // by name, and then by path.
+      //
+      for (auto c: pointer_result (
+             db.query<configuration> ("ORDER BY" +
+                                      query::default_ + "DESC," +
+                                      query::name     + "IS NULL," +
+                                      query::name     + "," +
+                                      query::path)))
+        cfgs.push_back (move (c));
+    }
+
+    t.commit ();
+
+
+    for (const shared_ptr<configuration>& c: cfgs)
+    {
+      //@@ TODO: use tabular layout facility when ready.
+
+      print_configuration (cout, c);
+      cout << endl;
+    }
+
+    return 0;
+  }
+
+  static int
   cmd_config_remove (const cmd_config_options& o, cli::scanner&)
   {
     tracer trace ("config_remove");
@@ -396,30 +457,30 @@ namespace bdep
       parse_command<cmd_config_subcommands> (scan,
                                              "config subcommand",
                                              "bdep help config"));
-
     // Validate options/subcommands.
     //
     if (const char* n = cmd_config_validate_add (o))
     {
-      if (!(c.add () || c.create () || c.set ()))
-        fail << n << " not valid for this command";
+      if (!c.add () && !c.create () && !c.set ())
+        fail << n << " not valid for this subcommand";
 
       if (o.wipe () && !c.create ())
-        fail << "--wipe is not valid for this command";
+        fail << "--wipe is not valid for this subcommand";
     }
 
     // --all
     //
     if (o.all ())
     {
-      if (!c.remove ())
-        fail << "--all not valid for this command";
+      if (!c.list () && !c.remove () && !c.set ())
+        fail << "--all not valid for this subcommand";
     }
 
     // Dispatch to subcommand function.
     //
     if (c.add    ()) return cmd_config_add    (o, scan);
     if (c.create ()) return cmd_config_create (o, scan);
+    if (c.list   ()) return cmd_config_list   (o, scan);
     if (c.remove ()) return cmd_config_remove (o, scan);
     if (c.rename ()) return cmd_config_rename (o, scan);
     if (c.set    ()) return cmd_config_set    (o, scan);
diff --git a/bdep/project.cxx b/bdep/project.cxx
index dacb921..0d8686f 100644
--- a/bdep/project.cxx
+++ b/bdep/project.cxx
@@ -46,7 +46,7 @@ namespace bdep
       for (const string& n: po.config_name ())
       {
         if (auto c = db.query_one<configuration> (query::name == n))
-          add (c);
+          add (move (c));
         else
           fail << "no configuration name '" << n << "' in project " << prj;
       }
@@ -62,7 +62,7 @@ namespace bdep
         d.normalize ();
 
         if (auto c = db.query_one<configuration> (query::path == d.string ()))
-          add (c);
+          add (move (c));
         else
           fail << "no configuration directory " << d << " in project " << prj;
       }
@@ -75,7 +75,7 @@ namespace bdep
       for (uint64_t id: po.config_id ())
       {
         if (auto c = db.find<configuration> (id))
-          add (c);
+          add (move (c));
         else
           fail << "no configuration id " << id << " in project " << prj;
       }
@@ -86,7 +86,7 @@ namespace bdep
     if (po.all ())
     {
       for (auto c: pointer_result (db.query<configuration> ()))
-        add (c);
+        add (move (c));
 
       if (r.empty ())
         fail << "no existing configurations";
@@ -99,7 +99,7 @@ namespace bdep
       if (fallback_default)
       {
         if (auto c = db.query_one<configuration> (query::default_))
-          add (c);
+          add (move (c));
         else
           fail << "no default configuration in project " << prj <<
             info << "use (@<cfg-name> | --config|-c <cfg-dir> | --all|-a) to "
diff --git a/tests/init.test b/tests/init.test
index 40e9500..14293ba 100644
--- a/tests/init.test
+++ b/tests/init.test
@@ -22,7 +22,7 @@ deinit += -d prj
 
   $* -C @cfg $cxx 'config.cc.poptions=-DTEST' 2>>/~"%EOE%" &prj-cfg/***;
     initializing in project $~/prj/
-    created configuration @cfg $~/prj-cfg/ \(1, default, forwarded, auto-synchronized\)
+    created configuration @cfg $~/prj-cfg/ 1 default,forwarded,auto-synchronized
     synchronizing:
     %  new prj.+19700101000000%
     EOE
@@ -70,14 +70,14 @@ deinit += -d prj
 
   $* -A @cfg1 2>>/~"%EOE%";
     initializing in project $~/prj/
-    added configuration @cfg1 $~/prj-cfg1/ \(1, default, forwarded, auto-synchronized\)
+    added configuration @cfg1 $~/prj-cfg1/ 1 default,forwarded,auto-synchronized
     synchronizing:
     %  new prj.+19700101000000%
     EOE
 
   $* -A prj-cfg2 @cfg2 2>>/~"%EOE%";
     initializing in project $~/prj/
-    added configuration @cfg2 $~/prj-cfg2/ \(2, auto-synchronized\)
+    added configuration @cfg2 $~/prj-cfg2/ 2 auto-synchronized
     synchronizing:
     %  new prj.+19700101000000%
     EOE
@@ -132,7 +132,7 @@ deinit += -d prj
 
   $* -C @cfg $cxx 2>>/~"%EOE%" &prj-cfg/***;
     initializing in project $~/prj/
-    created configuration @cfg $~/prj-cfg/ \(1, default, forwarded, auto-synchronized\)
+    created configuration @cfg $~/prj-cfg/ 1 default,forwarded,auto-synchronized
     synchronizing:
     %  new prj.+19700101000000%
     EOE
diff --git a/tests/new.test b/tests/new.test
index 99292b9..94886ad 100644
--- a/tests/new.test
+++ b/tests/new.test
@@ -84,7 +84,7 @@ status += -d prj
   {
     $* -C prj-config @cfg prj cc $cxx 2>>/~"%EOE%" &prj/*** &prj-config/***;
       created new executable project prj in $~/prj/
-      created configuration @cfg $~/prj-config/ \(1, default, forwarded, auto-synchronized\)
+      created configuration @cfg $~/prj-config/ 1 default,forwarded,auto-synchronized
       synchronizing:
       %  new prj.+19700101000000%
       EOE
@@ -109,7 +109,7 @@ status += -d prj
   {
     $* -C -@cfg prj cc $cxx 2>>/~"%EOE%" &prj/*** &prj-cfg/***;
       created new executable project prj in $~/prj/
-      created configuration @cfg $~/prj-cfg/ \(1, default, forwarded, auto-synchronized\)
+      created configuration @cfg $~/prj-cfg/ 1 default,forwarded,auto-synchronized
       synchronizing:
       %  new prj.+19700101000000%
       EOE
-- 
cgit v1.1