Skip to content

Add relative path method and normalize parent references #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add relative path method and normalize parent references #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion Sources/PathKit.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,13 @@ extension Path {
/// representation.
///
public func normalize() -> Path {
return Path(NSString(string: self.path).standardizingPath)
var path = Path(NSString(string: self.path).standardizingPath)
if !path.isAbsolute {
// standardizingPath only cleans up redundant ".." if the path is absolute. We can perform this normalization
// by recombining the path's components, since + will backtrack on parent directory references.
path = path.components.reduce(Path(), +)
}
return path
}

/// De-normalizes the path, by replacing the current user home directory with "~".
Expand Down Expand Up @@ -250,6 +256,60 @@ extension Path {

return pathExtension
}

/// Returns the relative path necessary to go from `base` to `self`.
///
/// Both paths must be absolute or relative paths.
/// - throws: Throws an error when the path types do not match, or when `base` has so many parent path components
/// that it refers to an unknown parent directory.
public func relativePath(from base: Path) throws -> Path {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public func relativePath(from base: Path) throws -> Path {
public func relative(from base: Path) throws -> Path {

What do you think about naming this relative(from:), I'm thinking about the consistency with abbreviate(), symlinkDestination(), symlink(_:), link(:), home, temporary etc for which the "Path" isn't explicitly in the name.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also slightly wondering if it might be better to do it the other way (which is how Python's pathlib has it PurePath.relative_to(*other)). I think it may be a bit easier to reason about and understand as a user of PathKit.

This is how you've instinctively written the tests and made a function which reversed the order.

Suggested change
public func relativePath(from base: Path) throws -> Path {
public func relative(to other: Path) throws -> Path {

enum PathArgumentError: Error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is specific for relative paths, should we name it as such?

Suggested change
enum PathArgumentError: Error {
enum RelativePathError: Error {

/// Can't back out of an unknown parent directory
case unknownParentDirectory
/// It's impossible to determine the path between an absolute and a relative path
case unmatchedAbsolutePath
}

func pathComponents(for path: ArraySlice<String>, relativeTo base: ArraySlice<String>, memo: [String]) throws -> [String] {
switch (base.first, path.first) {
// Base case: Paths are equivalent
case (.none, .none):
return memo

// No path to backtrack from
case (.none, .some(let rhs)):
guard rhs != "." else {
// Skip . instead of appending it
return try pathComponents(for: path.dropFirst(), relativeTo: base, memo: memo)
}
return try pathComponents(for: path.dropFirst(), relativeTo: base, memo: memo + [rhs])

// Both sides have a common parent
case (.some(let lhs), .some(let rhs)) where lhs == rhs:
return try pathComponents(for: path.dropFirst(), relativeTo: base.dropFirst(), memo: memo)

// `base` has a path to back out of
case (.some(let lhs), _):
guard lhs != ".." else {
throw PathArgumentError.unknownParentDirectory
}
guard lhs != "." else {
// Skip . instead of resolving it to ..
return try pathComponents(for: path, relativeTo: base.dropFirst(), memo: memo)
}
return try pathComponents(for: path, relativeTo: base.dropFirst(), memo: memo + [".."])
}
}

guard isAbsolute && base.isAbsolute || !isAbsolute && !base.isAbsolute else {
throw PathArgumentError.unmatchedAbsolutePath
}

return Path(components: try pathComponents(for: ArraySlice(normalize().components),
relativeTo: ArraySlice(base.normalize().components),
memo: []))
}

}


Expand Down
59 changes: 59 additions & 0 deletions Tests/PathKitTests/PathKitSpec.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,5 +509,64 @@ describe("PathKit") {
try expect(paths) == results.sorted(by: <)
}
}

$0.describe("relativePath(from:)") {
func relativePath(to path: String, from base: String) throws -> String {
return try Path(path).relativePath(from: Path(base)).string
}
Comment on lines +514 to +516
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path conforms to ExpressibleByStringLiteral I think that could allow us to use Path types here which automatically get handled from a string.

Here's some examples:

let path: Path = "a"

When passed into a function:

func relativePath(to path: Path, from base; Path) throws -> Path {
  return try path.relativePath(from: base)
}

relativePath(to: "a", from: "b") == "../a"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As extension we might be able to do away with this utility function and use tests directly:

- try expect(relativePath(to: "a", from: "b")) == "../a"
+ try expect(Path("b").relative(from: "a")) == "../a"


// These are based on ruby's tests for Pathname#relative_path_from:
// https://github.com/ruby/ruby/blob/7c2bbd1c7d40a30583844d649045824161772e36/test/pathname/test_pathname.rb#L297

$0.it("resolves single-level paths") {
try expect(relativePath(to: "a", from: "b")) == "../a"
try expect(relativePath(to: "a", from: "b/")) == "../a"
try expect(relativePath(to: "a/", from: "b")) == "../a"
try expect(relativePath(to: "a/", from: "b/")) == "../a"
try expect(relativePath(to: "/a", from: "/b")) == "../a"
try expect(relativePath(to: "/a", from: "/b/")) == "../a"
try expect(relativePath(to: "/a/", from: "/b")) == "../a"
try expect(relativePath(to: "/a/", from: "/b/")) == "../a"
}

$0.it("resolves paths with a common parent") {
try expect(relativePath(to: "a/b", from: "a/c")) == "../b"
try expect(relativePath(to: "../a", from: "../b")) == "../a"
}

$0.it("resolves dot paths") {
try expect(relativePath(to: "a", from: ".")) == "a"
try expect(relativePath(to: ".", from: "a")) == ".."
try expect(relativePath(to: ".", from: ".")) == "."
try expect(relativePath(to: "..", from: "..")) == "."
try expect(relativePath(to: "..", from: ".")) == ".."
}

$0.it("resolves multi-level paths") {
try expect(relativePath(to: "/a/b/c/d", from: "/a/b")) == "c/d"
try expect(relativePath(to: "/a/b", from: "/a/b/c/d")) == "../.."
try expect(relativePath(to: "/e", from: "/a/b/c/d")) == "../../../../e"
try expect(relativePath(to: "a/b/c", from: "a/d")) == "../b/c"
try expect(relativePath(to: "/../a", from: "/b")) == "../a"
try expect(relativePath(to: "../a", from: "b")) == "../../a"
try expect(relativePath(to: "/a/../../b", from: "/b")) == "."
try expect(relativePath(to: "a/..", from: "a")) == ".."
try expect(relativePath(to: "a/../b", from: "b")) == "."
}

$0.it("backtracks on a non-normalized base path") {
try expect(relativePath(to: "a", from: "b/..")) == "a"
try expect(relativePath(to: "b/c", from: "b/..")) == "b/c"
}

$0.it("throws when given unresolvable paths") {
try expect(relativePath(to: "/", from: ".")).toThrow()
try expect(relativePath(to: ".", from: "/")).toThrow()
try expect(relativePath(to: "a", from: "..")).toThrow()
try expect(relativePath(to: ".", from: "..")).toThrow()
try expect(relativePath(to: "a", from: "b/../..")).toThrow()
}
}

}
}