Add path::combine(string,separator)

In particular, this can be used to recombine a path during iteration:

  dir_path r;
  for (auto i (d.begin ()); i != d.end (); ++i)
    r.combine (*i, i.separator ());

3 files changed, 106 insertions, 13 deletions

diff --git a/libbutl/path.ixx b/libbutl/path.ixx
index 27cba2b..8512286 100644
--- a/libbutl/path.ixx
+++ b/libbutl/path.ixx
@@ -544,9 +544,9 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
 
   template <typename C, typename K>
   inline void basic_path<C, K>::
-  combine (const C* r, size_type rn, difference_type rts)
+  combine_impl (const C* r, size_type rn, difference_type rts)
   {
-    //assert (rn != 0);
+    //assert (rn != 0); // Should be ensured by caller.
 
     string_type& l (this->path_);
     difference_type& ts (this->tsep_);
@@ -566,7 +566,7 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
 
   template <typename C, typename K>
   inline void basic_path<C, K>::
-  combine (const C* r, size_type rn)
+  combine_impl (const C* r, size_type rn)
   {
     // If we do (dir_path / path) then we will end up with path. What should
     // we end up if we do (dir_path / "foo") vs (dir_path / "foo/")? We cannot
@@ -575,12 +575,12 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
     // dir_path gets the canonical trailing slash if one wasn't there.
     //
     // For now we won't allow the slash and will always add the canonical one
-    // for dir_path (via cast()).
+    // for dir_path (via cast()). But also see the public combine() functions.
     //
     if (traits_type::find_separator (r, rn) != nullptr)
-      throw invalid_basic_path<C> (r);
+      throw invalid_basic_path<C> (r, rn);
 
-    combine (r, rn, 0);
+    combine_impl (r, rn, 0);
     K::cast (*this);
   }
 
@@ -592,7 +592,7 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
       throw invalid_basic_path<C> (r.path_);
 
     if (!r.empty ())
-      combine (r.path_.c_str (), r.path_.size (), r.tsep_);
+      combine_impl (r.path_.c_str (), r.path_.size (), r.tsep_);
 
     return *this;
   }
@@ -602,7 +602,7 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
   operator/= (string_type const& r)
   {
     if (size_type rn = r.size ())
-      combine (r.c_str (), rn);
+      combine_impl (r.c_str (), rn);
 
     return *this;
   }
@@ -612,13 +612,55 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
   operator/= (const C* r)
   {
     if (size_type rn = string_type::traits_type::length (r))
-      combine (r, rn);
+      combine_impl (r, rn);
 
     return *this;
   }
 
   template <typename C, typename K>
   inline void basic_path<C, K>::
+  combine (string_type const& r, C s)
+  {
+    combine (r.c_str (), r.size (), s);
+  }
+
+  template <typename C, typename K>
+  inline void basic_path<C, K>::
+  combine (const C* r, C s)
+  {
+    combine (r, string_type::traits_type::length (r), s);
+  }
+
+  template <typename C, typename K>
+  inline void basic_path<C, K>::
+  combine (const C* r, size_type rn, C s)
+  {
+    if (rn != 0 || s != '\0')
+    {
+      if (traits_type::find_separator (r, rn) != nullptr)
+        throw invalid_basic_path<C> (r, rn);
+
+#ifndef _WIN32
+      if (rn == 0 && empty ()) // POSIX root.
+      {
+        this->path_ += s;
+        this->tsep_ = -1;
+        return;
+      }
+#endif
+
+      if (rn != 0)
+        combine_impl (r, rn, 0);
+
+      if (s != '\0')
+        this->tsep_ = traits_type::separator_index (s);
+
+      K::cast (*this);
+    }
+  }
+
+  template <typename C, typename K>
+  inline void basic_path<C, K>::
   append (const C* r, size_type rn)
   {
     //assert (this->tsep_ != -1); // Append to root?
diff --git a/libbutl/path.mxx b/libbutl/path.mxx
index 136f361..1ee2a66 100644
--- a/libbutl/path.mxx
+++ b/libbutl/path.mxx
@@ -72,11 +72,15 @@ LIBBUTL_MODEXPORT namespace butl
   struct invalid_basic_path: invalid_path_base
   {
     using string_type = std::basic_string<C>;
+    using size_type = typename string_type::size_type;
 
     string_type path;
 
-    invalid_basic_path (const C* p): path (p) {}
+    explicit
     invalid_basic_path (const string_type& p): path (p) {}
+    explicit
+    invalid_basic_path (const C* p): path (p) {}
+    invalid_basic_path (const C* p, size_type n): path (p, n) {}
   };
 
   enum class path_abnormality: std::uint16_t
@@ -943,6 +947,16 @@ LIBBUTL_MODEXPORT namespace butl
 
     // Iteration over path components.
     //
+    // Note that for an absolute POSIX path the first component is empty,
+    // not `/`. Which means recombining a path with operator/= is not going
+    // to work. Instead, do something along these lines:
+    //
+    // dir_path r;
+    // for (auto i (d.begin ()); i != d.end (); ++i)
+    //   r.combine (*i, i.separator ());
+    //
+    // @@ TODO: would be nice to skip consecutive separators (foo//bar).
+    //
   public:
     struct iterator
     {
@@ -1022,6 +1036,8 @@ LIBBUTL_MODEXPORT namespace butl
       iterator
       operator-- (int) {iterator r (*this); operator-- (); return r;}
 
+      // @@ TODO: this should return string_view.
+      //
       string_type
       operator* () const
       {
@@ -1137,11 +1153,14 @@ LIBBUTL_MODEXPORT namespace butl
     realize ();
 
   public:
+    // Combine two paths. Note: empty path on RHS has no effect.
+    //
     basic_path&
     operator/= (basic_path const&);
 
     // Combine a single path component (must not contain directory separators)
-    // as a string, without first constructing the path object.
+    // as a string, without first constructing the path object. Note: empty
+    // string has no effect.
     //
     basic_path&
     operator/= (string_type const&);
@@ -1149,6 +1168,19 @@ LIBBUTL_MODEXPORT namespace butl
     basic_path&
     operator/= (const C*);
 
+    // As above but with an optional separator after the component. Note that
+    // if the LHS is empty and the string is empty but the separator is not
+    // '\0', then on POSIX this is treated as a root component.
+    //
+    void
+    combine (string_type const&, C separator);
+
+    void
+    combine (const C*, C separator);
+
+    void
+    combine (const C*, size_type, C separator);
+
     // Append to the end of the path (normally an extension, etc).
     //
     basic_path&
@@ -1240,10 +1272,10 @@ LIBBUTL_MODEXPORT namespace butl
     // Common implementation for operator/=.
     //
     void
-    combine (const C*, size_type, difference_type);
+    combine_impl (const C*, size_type, difference_type);
 
     void
-    combine (const C*, size_type);
+    combine_impl (const C*, size_type);
 
     // Friends.
     //
diff --git a/tests/path/driver.cxx b/tests/path/driver.cxx
index 28da0a0..b855e34 100644
--- a/tests/path/driver.cxx
+++ b/tests/path/driver.cxx
@@ -208,6 +208,15 @@ main ()
     assert (++i != p.end () && *i == "bar" && i.separator () == '\0');
     assert (++i == p.end ());
   }
+  /*
+  {
+    path p ("foo//bar");
+    path::iterator i (p.begin ());
+    assert (i != p.end () && *i == "foo" && i.separator () == '/');
+    assert (++i != p.end () && *i == "bar" && i.separator () == '\0');
+    assert (++i == p.end ());
+  }
+  */
   {
     path p ("foo/bar/");
     path::iterator i (p.begin ());
@@ -251,6 +260,15 @@ main ()
     assert (i != p.rend () && *i == "");
     assert (++i == p.rend ());
   }
+#else
+  {
+    path p ("C:\\foo\\bar");
+    path::iterator i (p.begin ());
+    assert (i != p.end () && *i == "C:");
+    assert (++i != p.end () && *i == "foo");
+    assert (++i != p.end () && *i == "bar");
+    assert (++i == p.end ());
+  }
 #endif
 
   // iterator range construction
@@ -318,6 +336,7 @@ main ()
   assert ((path ("foo/") / path ()).representation () == "foo/");
 #else
   assert ((path ("C:\\") / path ("tmp")).representation () == "C:\\tmp");
+  assert ((path ("C:") / path ("tmp")).representation () == "C:\\tmp");
   assert ((path ("foo\\") / path ("bar")).representation () == "foo\\bar");
   assert ((path ("foo\\") / path ("bar\\")).representation () == "foo\\bar\\");
   assert ((path ("foo\\") / path ("bar/")).representation () == "foo\\bar/");