finish up make_relative

Author: jamestrew

lua/plenary/path2.lua

@@ -17,8 +17,18 @@
 ---   things like `path.filename = 'foo'` but now explicitly adding some barrier
 ---   to this. Allows us to compute `filename` from "metadata" parsed once on
 ---   instantiation.
-
---- TODO: rework `_filename` according to `_format_parsed_parts`
+---
+--- - FIX: `Path:make_relative` throws error if you try to make a path relative
+---   to another path that is not in the same subpath.
+---
+---   eg. `Path:new("foo/bar_baz"):make_relative("foo/bar")` => errors as you
+---   can't get to "foo/bar_baz" from "foo/bar" without going up in directory.
+---   This would previously return "foo/bar_baz" which is wrong.
+---
+---   Adds an option to walk up path to compensate.
+---
+---   eg. `Path:new("foo/bar_baz"):make_relative("foo/bar", true)` => returns
+---   "../bar_baz"
 
 local uv = vim.loop
 local iswin = uv.os_uname().sysname == "Windows_NT"
@@ -609,8 +619,21 @@ function Path:is_relative(to)
   end
   ---@cast to plenary.Path2
 
+  if to == self then
+    return true
+  end
+
+  -- NOTE: could probably be optimized by letting _WindowsPath/_WindowsPath
+  -- handle this.
+
   local to_abs = to:absolute()
-  return self:absolute():sub(1, #to_abs) == to_abs
+  for parent in self:iter_parents() do
+    if to_abs == parent then
+      return true
+    end
+  end
+
+  return false
 end
 
 --- makes a path relative to another (by default the cwd).
@@ -620,6 +643,11 @@ end
 ---@param walk_up boolean? walk up to the provided path using '..' (default: `false`)
 ---@return string
 function Path:make_relative(to, walk_up)
+  -- NOTE: could probably take some shortcuts and avoid some `Path:new` calls
+  -- by allowing _WindowsPath/_PosixPath handle this individually.
+  -- As always, Windows root complicates things, so generating a new Path often
+  -- easier/less error prone than manual string manipulate but at the cost of
+  -- perf.
   walk_up = vim.F.if_nil(walk_up, false)
 
   if to == nil then
@@ -649,7 +677,6 @@ function Path:make_relative(to, walk_up)
   local common_path
   for parent in to:iter_parents() do
     table.insert(steps, "..")
-    print(parent, abs)
     if abs:sub(1, #parent) == parent then
       common_path = parent
       break
@@ -660,18 +687,9 @@ function Path:make_relative(to, walk_up)
     error(string.format("'%s' and '%s' have different anchors", self, to))
   end
 
-  local res_path = abs:sub(#common_path + 1)
-  return table.concat(steps, self.sep) .. res_path
+  local res_path = abs:sub(#common_path + 1):gsub("^" .. self.sep, "")
+  table.insert(steps, res_path)
+  return Path:new(steps).filename
 end
 
--- vim.o.shellslash = false
-
-local root = "C:/"
-local p = Path:new(Path.path.root(vim.fn.getcwd()))
-vim.print("p parent", p.filename, p:parents(), p:parent().filename)
--- local absolute = p:absolute()
--- local relative = Path:new(absolute):make_relative(Path:new "C:/Windows", true)
--- print(p.filename, absolute, relative)
-vim.o.shellslash = true
-
 return Path

tests/plenary/path2_spec.lua

@@ -307,32 +307,31 @@ describe("Path2", function()
       assert.are.same(p.filename, relative)
     end)
 
-    -- it_cross_plat("can take absolute paths and make them relative to the root directory", function()
-    --   local p = Path:new { root, "prime", "aoeu", "agen.lua" }
-    --   local absolute = root .. p.filename
-    --   local relative = Path:new(absolute):make_relative(root)
-    --   assert.are.same(p.filename, relative)
-    -- end)
+    it_cross_plat("can take absolute paths and make them relative to the root directory", function()
+      local p = Path:new { root, "prime", "aoeu", "agen.lua" }
+      local absolute = root .. p.filename
+      local relative = Path:new(absolute):make_relative(root)
+      assert.are.same(p.filename, relative)
+    end)
 
     it_cross_plat("can take absolute paths and make them relative to themselves", function()
       local p = Path:new { root, "home", "prime", "aoeu", "agen.lua" }
       local relative = Path:new(p.filename):make_relative(p.filename)
       assert.are.same(".", relative)
     end)
 
-    -- it_cross_plat("should not truncate if path separator is not present after cwd", function()
-    --   local cwd = "tmp" .. path.sep .. "foo"
-    --   local p = Path:new { "tmp", "foo_bar", "fileb.lua" }
-    --   local relative = Path:new(p.filename):make_relative(cwd)
-    --   assert.are.same(p.filename, relative)
-    -- end)
-
-    -- it_cross_plat("should not truncate if path separator is not present after cwd and cwd ends in path sep", function()
-    --   local cwd = "tmp" .. path.sep .. "foo" .. path.sep
-    --   local p = Path:new { "tmp", "foo_bar", "fileb.lua" }
-    --   local relative = Path:new(p.filename):make_relative(cwd)
-    --   assert.are.same(p.filename, relative)
-    -- end)
+    it_cross_plat("should fail to make relative a path to somewhere not in the subpath", function()
+      assert.has_error(function()
+        _ = Path:new({ "tmp", "foo_bar", "fileb.lua" }):make_relative(Path:new { "tmp", "foo" })
+      end)
+    end)
+
+    it_cross_plat("can walk upwards out of current subpath", function()
+      local p = Path:new { "foo", "bar", "baz" }
+      local cwd = Path:new { "foo", "foo_inner" }
+      local expect = Path:new { "..", "bar", "baz" }
+      assert.are.same(expect.filename, p:make_relative(cwd, true))
+    end)
   end)
 
   describe("parents", function()