-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Change relative path calculation #6311
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.
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1031,27 +1031,72 @@ internal static string MakeRelative(string basePath, string path) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ErrorUtilities.VerifyThrowArgumentNull(basePath, nameof(basePath)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ErrorUtilities.VerifyThrowArgumentLength(path, nameof(path)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (basePath.Length == 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string fullBase = Path.GetFullPath(basePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string fullPath = Path.GetFullPath(path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Uri baseUri = new Uri(EnsureTrailingSlash(basePath), UriKind.Absolute); // May throw UriFormatException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string[] splitBase = fullBase.Split(new char[] { Path.DirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string[] splitPath = fullPath.Split(new char[] { Path.DirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Uri pathUri = CreateUriFromPath(path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ErrorUtilities.VerifyThrow(splitPath.Length > 0, "Cannot call MakeRelative on a path of only slashes."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!pathUri.IsAbsoluteUri) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // On a mac, the path could start with any number of slashes and still be valid. We have to check them all. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int indexOfFirstNonSlashChar = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (path[indexOfFirstNonSlashChar] == Path.DirectorySeparatorChar) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // the path is already a relative url, we will just normalize it... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pathUri = new Uri(baseUri, pathUri); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| indexOfFirstNonSlashChar++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (path.IndexOf(splitPath[0]) != indexOfFirstNonSlashChar) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // path was already relative so just return it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return FixFilePath(path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Uri relativeUri = baseUri.MakeRelativeUri(pathUri); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string relativePath = Uri.UnescapeDataString(relativeUri.IsAbsoluteUri ? relativeUri.LocalPath : relativeUri.ToString()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int baseI = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int pathI = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit (last one, promise!): Since the two integers are always incremented together, consider having only one and changing the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep! This PR came out unusually janky, so I appreciate all the refactoring suggestions. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (baseI == splitBase.Length) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pathI == splitPath.Length) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
ladipro marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else if (pathI == splitPath.Length) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else if (splitBase[baseI].Equals(splitPath[pathI], PathComparison)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| baseI++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pathI++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would find the following much easier to read. As long as it's equivalent, please double-check:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string result = relativePath.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| StringBuilder sb = StringBuilderCache.Acquire(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If the paths have no component in common, the only valid relative path is the full path. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hyper-nit-9000: newline here.
Forgind marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (baseI == 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
benvillalobos marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fullPath; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (baseI < splitBase.Length) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sb.Append("..").Append(Path.DirectorySeparatorChar); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| baseI++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (pathI < splitPath.Length) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sb.Append(splitPath[pathI]).Append(Path.DirectorySeparatorChar); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pathI++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sb.Remove(sb.Length - 1, 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I would expect
Suggested change
to be slightly faster and also easier to read. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return StringBuilderCache.GetStringAndRelease(sb); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The char array is worth caching. And looks like we already do it in
Constants.DirectorySeparatorChar!