Skip to content

Add new options for formatting relative include paths #47

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 new options for formatting relative include paths #47

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
4 changes: 2 additions & 2 deletions IncludeToolbox.vsix
View file
Open in desktop
Git LFS file not shown
80 changes: 73 additions & 7 deletions IncludeToolbox/Formatter/IncludeFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@ namespace IncludeToolbox.Formatter
{
public static class IncludeFormatter
{
public static string FormatPath(string absoluteIncludeFilename, FormatterOptionsPage.PathMode pathformat, IEnumerable<string> includeDirectories)
public static string FormatPath(string absoluteIncludeFilename,
FormatterOptionsPage.PathMode pathformat,
IEnumerable<string> includeDirectories,
string includeRootDirectory = null)
{
if (pathformat == FormatterOptionsPage.PathMode.Absolute)
{
return absoluteIncludeFilename;
}
else if (pathformat == FormatterOptionsPage.PathMode.Absolute_FromParentDirWithFile &&
null != absoluteIncludeFilename &&
null != includeRootDirectory)
{
return Utils.MakeRelative(includeRootDirectory, absoluteIncludeFilename);
}
else
{
// todo: Treat std library files special?
Expand Down Expand Up @@ -47,16 +56,44 @@ public static string FormatPath(string absoluteIncludeFilename, FormatterOptions
/// <summary>
/// Formats the paths of a given list of include line info.
/// </summary>
private static void FormatPaths(IEnumerable<IncludeLineInfo> lines, FormatterOptionsPage.PathMode pathformat, IEnumerable<string> includeDirectories)
private static void FormatPaths(IEnumerable<IncludeLineInfo> lines,
FormatterOptionsPage.PathMode pathformat,
FormatterOptionsPage.IgnoreFileRelativeMode ignoreFileRelativeMode,
IEnumerable<string> includeDirectories,
string documentDir,
string includeRootDirectory = null)
{
if (pathformat == FormatterOptionsPage.PathMode.Unchanged)
return;

foreach (var line in lines)
{
string absoluteIncludeDir = line.TryResolveInclude(includeDirectories, out bool resolvedPath);
string absoluteIncludePath = line.TryResolveInclude(includeDirectories, out bool resolvedPath);
if (resolvedPath)
line.IncludeContent = FormatPath(absoluteIncludeDir, pathformat, includeDirectories) ?? line.IncludeContent;
{
if (pathformat == FormatterOptionsPage.PathMode.Absolute_FromParentDirWithFile &&
includeRootDirectory != null &&
!absoluteIncludePath.StartsWith(includeRootDirectory))
{
continue;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Avoids files outside of the root directory tree (e.g. system includes)

}

var currentPathFormat = pathformat;
var currentIncludeRootDirectory = includeRootDirectory;
string includeFilename = Path.GetFileName(absoluteIncludePath);
if ((ignoreFileRelativeMode == FormatterOptionsPage.IgnoreFileRelativeMode.InSameDirectory &&
Path.Combine(documentDir, includeFilename) == absoluteIncludePath) ||
(ignoreFileRelativeMode == FormatterOptionsPage.IgnoreFileRelativeMode.InSameOrSubDirectory &&
absoluteIncludePath.StartsWith(documentDir)))
{
currentPathFormat = FormatterOptionsPage.PathMode.Absolute_FromParentDirWithFile;
currentIncludeRootDirectory = documentDir;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For these IgnoreFileRelativeModes, we need to change the path format and root directory to root the include path appropriately.

Copy link
Owner

Choose a reason for hiding this comment

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

Not very happy with this. The combination of FormatterOptionsPage.IgnoreFileRelativeMode.InSameDirectory/InSameDirectoryOrSubdirectory with FormatterOptionsPage.PathMode.Shortest would no longer find the shortest possible path!
We need to pass on the FormatterOptionsPage.IgnoreFileRelativeMode to the inner FormatPath to be able to find the best path according to the PathMode

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll give that a try. I worry a bit that some options will be at cross-purposes with one another in some configurations (like "why would you do that?" configurations). If it makes things less awkward, though, I'm all for it!

}
line.IncludeContent = FormatPath(absoluteIncludePath,
Copy link
Owner

Choose a reason for hiding this comment

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

Old version was guarded against FormatPath returning null. There might have been a good reason for that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It still is, just a few lines down because of how I formatted the parameters. (I dislike commenting on code in GitHub because I find inline comments hard to place well and hard to read -- I'd rather them show up in some kind of side bar so they don't split up the actual code!)

currentPathFormat,
includeDirectories,
currentIncludeRootDirectory) ?? line.IncludeContent;
}
}
}

Expand Down Expand Up @@ -225,8 +262,32 @@ private static bool SortIncludeBatch(FormatterOptionsPage settings, string[] pre
/// <returns>Formated text.</returns>
public static string FormatIncludes(string text, string documentPath, IEnumerable<string> includeDirectories, FormatterOptionsPage settings)
{
string documentDir = Path.GetDirectoryName(documentPath);
string documentDir = Utils.GetExactPathName(Path.GetDirectoryName(documentPath));
string documentName = Path.GetFileNameWithoutExtension(documentPath);
string includeRootDirectory = null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The following loops up through parent directories looking for the file specified in the settings.

if (settings.PathFormat == FormatterOptionsPage.PathMode.Absolute_FromParentDirWithFile &&
!String.IsNullOrWhiteSpace(settings.FromParentDirWithFile))
{
var dir = new DirectoryInfo(documentDir);
while (dir != null)
{
if (File.Exists(Path.Combine(dir.FullName, settings.FromParentDirWithFile)))
{
includeRootDirectory = Utils.GetExactPathName(dir.FullName);
break;
}

try
{
dir = dir.Parent;
}
catch (System.Security.SecurityException)
{
// Permission denied
break;
}
}
}

includeDirectories = new string[] { Microsoft.VisualStudio.PlatformUI.PathUtil.Normalize(documentDir) + Path.DirectorySeparatorChar }.Concat(includeDirectories);

Expand All @@ -236,11 +297,16 @@ public static string FormatIncludes(string text, string documentPath, IEnumerabl

// Format.
IEnumerable<string> formatingDirs = includeDirectories;
if (settings.IgnoreFileRelative)
if (settings.IgnoreFileRelative == FormatterOptionsPage.IgnoreFileRelativeMode.Always)
{
formatingDirs = formatingDirs.Skip(1);
}
FormatPaths(lines, settings.PathFormat, formatingDirs);
FormatPaths(lines,
settings.PathFormat,
settings.IgnoreFileRelative,
formatingDirs,
documentDir,
includeRootDirectory);
FormatDelimiters(lines, settings.DelimiterFormatting);
FormatSlashes(lines, settings.SlashFormatting);

Expand Down
24 changes: 20 additions & 4 deletions IncludeToolbox/Options/FormatterOptionsPage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,29 @@ public enum PathMode
Shortest,
Shortest_AvoidUpSteps,
Absolute,
Absolute_FromParentDirWithFile
Copy link
Owner

Choose a reason for hiding this comment

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

This not actually absolute in any sense. It is relative from where the special file is placed, so I'd call it ForceRelativeToParentDirWithFile since it will always try to make the path relative to the special folder, blindly assuming that this will compile. In contrast all the other modes ensure that the the path can be resolved with the IncludePaths in the project file or by being relative to the including source file.

Please correct me if I got this wrong :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, you're right. I think your name will make more sense. In our workflow I just tend to think about them as "absolute with respect to the code base" vs. "relative to the current file".

}
[Category("Path")]
[DisplayName("Mode")]
[Description("Changes the path mode to the given pattern.")]
public PathMode PathFormat { get; set; } = PathMode.Shortest_AvoidUpSteps;

[Category("Path")]
[DisplayName("Filename in parent directory for absolute include paths")]
[Description("The Absolute_FromParentDirWithFile mode will look for this file in parent directories and make include paths absolute from its location.")]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This functionality is somewhat difficult to describe...

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah I never heard of this whole thing and it took me a while indeed. I think your description here is quite good after all though.

Suggestion for a slight improvement:

[DisplayName("Filename for Absolute_FromParentDirWithFile")]
[Description("The Absolute_FromParentDirWithFile mode will look for this file in all parent directories and make include paths absolute from its location if found.")]

public string FromParentDirWithFile { get; set; } = "build.root";

public enum IgnoreFileRelativeMode
Copy link
Owner

Choose a reason for hiding this comment

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

Originally this was intended to suppress the "natural include path" even if that one would be better according to the policy in PathMode.
You're right, this is all not too intuitive. But I think with your extension we completely lost the meaning of "ignore" (never a good description of what was going on). So maybe this would be more concise?

enum AllowRelativePathMode
{
Always, // All relative paths are taken into consideration.
Never, // No relative path is taken into consideration.
OnlyInSameDirectory, // Relative paths are only allowed if they are in the very same directory than the including file.
OnlyInSameOrSubDirectory // Relative paths are allowed if they are in the same directory than the including file or any subdirectory.
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it might be a better idea to do something different. What do you think about this:

Remove Always/Never

  • To resolve the full path to the include, we should automatically use the including file's directory if the delimiters are quotes (https://msdn.microsoft.com/en-us/library/36k2cdd4.aspx)
    • This would go well with what I'd like to do next: Properly handle system includes (adding an Auto to DelimiterFormatting and handling their formatting specially (there's a TODO for something like this already))
  • This mode will instead optionally force use of paths relative to the including file's directory, overriding PathFormat if set
    • I really need this behavior -- I want to ForceRelativeToParentDirWithFile unless the include is a system one or in the same directory as the including file
  • If this mode is disabled, actually formatting include paths as relative to anything else will be left to the PathFormat

Basically I'm envisioning these two being like this:

public enum PathMode
{
    Unchanged,
    Shortest,               // Maybe for these we can check to see if there's a shorter one than the
    Shortest_AvoidUpSteps,  // force-file-relative mode instead of overriding them entirely 
    Absolute,
    ForceRelativeToParentDirWithFile
}

public enum ForceFileRelativePathMode
{
    Disabled,               // Off
    IfInSameDirectory,      // DEFAULT. Includes found in the directory containing the including
                            // file are always made relative to it. Does not include subdirectories.
    IfInSameOrSubDirectory  // Includes found in the directory containing the including file are
                            // always made relative to it. Includes subdirectories.
}

Copy link
Owner

Choose a reason for hiding this comment

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

I think you're definitely right on the resolve strategy! Didn't know about "2" for '"' delimiters. Taking this into account would be hard ...

I also agree on most of the rest as well but, as you're pointing out neither Shortest, Shortest_AvoidUpSteps, nor ForceRelativeToParentDirWithFile shouldn't be too much intimidated by ForceFileRelativePathMode. That being said we don't really force relative paths. Also, your suggested setup does not make it clear what you need to do to allow relative paths going outside of the source dir, e.g. "../header.h".
It seems all our modes strive "use the shortest out of the following", only that the list to choose from is differently every time. Looking at things from that perspective we would end up with various bit flags that govern which path are legal for either <> or "". But since this doesn't work with the UI, it might translate to this:

public enum PathFormattingMode
{
    Unchanged,
    Shortest, // default
    ForceAbsolute // absolute, no matter what. for debugging mainly
}
// Relative paths are never considered for includes using <>
public enum PathPool_FileRelativePath
{
    DontConsider, // pretend relative paths are not possible
    Consider, // add relative path like a normal include path
    ConsiderOnlyIfInSameDirectory, // Allow relative only if in same directory
    ConsiderOnlyIfInSameOrSubDirectory, // Allow relative only if in same directory or subdirectory
}
public enum PathPool_ProjectIncludePath
{
   AlwaysConsider // default
   ConsiderOnlyForAngled, // allows you to use enforce using relative or RelativeToParentDirWithFile for quoted ones
}
public enum PathPool_RelativeToParentDirWithFile
{
   DontConsider,
   AlwaysConsider,
   DontConsiderForAngled
}
public bool PathPool_AllowUpSteps = false; // If true allow "../" in directories

If I got everything right that would mean that your new desired mode is configured as Shortest, PathPool_FileRelativePath==ConsiderOnlyIfInSameDirectory , PathPool_ProjectIncludePath==ConsiderOnlyForAngled, PathPool_RelativeToParentDirWithFile==DontConsiderForAngled, PathPool_AllowUpSteps==false
Yeah I know, is this really better? Just trying to find flexible and easy to understand framework that fits everything :-/

About the delimiter thing: Having auto mode sounds great! Just don't rely on any semantic for existing/targeted delimiters, I worked in projects where all delimiters were angle brackets by convention.

Copy link
Collaborator Author

@dakotahawkins dakotahawkins Sep 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

With respect to Shortest and my workflow, I actually want "as long as it needs to be, starting from the root of our repository", but only for files not in the current project (and not for system files, of course).

I've already seen a couple of cases in our codebase where files that are in the current directory had longer relative paths that my current implementation fixed appropriately.

As an example, consider:

  • RepoDir/
    • build.root
    • Src/
      • MainProj/
        • MainProj.cpp
        • MainProj.h
        • MainUtility.h
        • stdafx.h
      • ProjA/
        • A_1.h
        • A_2.h
      • ProjB/
        • B_1.h
        • B_2.h

Group regexes:

(?i)stdafx\.h(?-i)                          // PCH
"(?i)$(currentFilename)\.(h|hpp|hxx)(?-i)"  // Corresponding header
<[^.]*\.                                    // C system includes
<                                           // C++ system includes
(\\|/)                                      // Other projects' headers
                                            // The rest (this project's headers)

Original:

// MainProj.cpp
// AdditionalIncludeDirs="../ProjA;../ProjB;$(BuildRootDirMacro);..."

#include "stdafx.h"

#include "A_2.h"
#include "B_1.h"
#include "B_2.h"
#include "MainProj.h"
#include "../Src/ProjA/A_1.h"
#include "../MainProj/MainUtility.h"

#include <vector>
#include <windows.h>
#include <string>

Formatted:

// MainProj.cpp
// AdditionalIncludeDirs="../ProjA;../ProjB;$(BuildRootDirMacro);..."

#include "stdafx.h"

#include "MainProj.h"

#include <windows.h>

#include <vector>
#include <string>

#include "Src/ProjA/A_1.h"
#include "Src/ProjA/A_2.h"
#include "Src/ProjB/B_1.h"
#include "Src/ProjB/B_2.h"

#include "MainUtility.h"

If you're curious, that grouping/order comes from the Google C++ Style Guide, which is what our style guide is based on. I have a few reasons for wanting those long, rooted paths:

  1. Standardization. It's super-obvious where each header actually lives.
  2. Easy searching
  3. Easy parsing with other tools! If you don't have to worry about digging include dirs out of the project files and property sheets, parsing includes becomes much easier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In terms of "paths going outside of the source dir", I'll admit that's not a case that affects me personally so I wasn't terribly concerned about it, but I think failing to fit other strategies and falling back on one of the Shortest options or even Unchanged may cover it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really think the wiki page would be good to sort this out, since my intention was to take a step back from the implementation and address what kind of general behavior we'd like to support.

Also, with so many options that affect one another, the possible behaviors really scale. It might be OK if there are some combinations of them that don't make sense and produce bad/silly results as long as those combinations aren't easy to accidentally have and the "good" combinations allow lots of kinds of results that people might actually want or need.

{
Never,
Always,
InSameDirectory,
InSameOrSubDirectory
}
[Category("Path")]
[DisplayName("Ignore File Relative")]
[Description("If true, include directives will not take the path of the file into account.")]
public bool IgnoreFileRelative { get; set; } = false;
[Description("Whether to ignore include paths relative to the current file.")]
public IgnoreFileRelativeMode IgnoreFileRelative { get; set; } = IgnoreFileRelativeMode.Never;

//[Category("Path")]
//[DisplayName("Ignore Standard Library")]
Expand Down Expand Up @@ -115,7 +128,8 @@ public override void SaveSettingsToStorage()
settingsStore.CreateCollection(collectionName);

settingsStore.SetInt32(collectionName, nameof(PathFormat), (int)PathFormat);
settingsStore.SetBoolean(collectionName, nameof(IgnoreFileRelative), IgnoreFileRelative);
settingsStore.SetString(collectionName, nameof(FromParentDirWithFile), FromParentDirWithFile);
settingsStore.SetInt32(collectionName, nameof(IgnoreFileRelative), (int)IgnoreFileRelative);

settingsStore.SetInt32(collectionName, nameof(DelimiterFormatting), (int)DelimiterFormatting);
settingsStore.SetInt32(collectionName, nameof(SlashFormatting), (int)SlashFormatting);
Expand All @@ -135,8 +149,10 @@ public override void LoadSettingsFromStorage()

if (settingsStore.PropertyExists(collectionName, nameof(PathFormat)))
PathFormat = (PathMode)settingsStore.GetInt32(collectionName, nameof(PathFormat));
if (settingsStore.PropertyExists(collectionName, nameof(FromParentDirWithFile)))
FromParentDirWithFile = settingsStore.GetString(collectionName, nameof(FromParentDirWithFile));
if (settingsStore.PropertyExists(collectionName, nameof(IgnoreFileRelative)))
IgnoreFileRelative = settingsStore.GetBoolean(collectionName, nameof(IgnoreFileRelative));
IgnoreFileRelative = (IgnoreFileRelativeMode)settingsStore.GetInt32(collectionName, nameof(IgnoreFileRelative));
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't checked - what happens if the bool was already saved and were loading an integer now with the same identifier?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That just happened to me accidentally. The field is blank :(

I guess I could add conversion code, but hacking that in seems like a slippery slope in the event there are more changes like that in the future. What do you think the right thing to do is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, it only doesn't work going backwards in version. Going forwards I think it works because the first two enum values correspond to the original boolean values. Of course, that will change if we flip the meaning of the setting.

Also changing the name of the setting, however, should just give you default values. So, not ideal for users that had non-defaults before but maybe not too terrible either.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah let's not over-think this (too). Let's go with a different name, then we're on the safe side.


if (settingsStore.PropertyExists(collectionName, nameof(DelimiterFormatting)))
DelimiterFormatting = (DelimiterMode) settingsStore.GetInt32(collectionName, nameof(DelimiterFormatting));
Expand Down
2 changes: 1 addition & 1 deletion IncludeToolbox/Package/source.extension.vsixmanifest
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
<Metadata>
<Identity Id="IncludeToolbox.Andreas Reich.075c2e2b-7b71-45ba-b2e6-c1dadc81cfac" Version="2.2.1" Language="en-US" Publisher="Andreas Reich" />
<Identity Id="IncludeToolbox.Andreas Reich.075c2e2b-7b71-45ba-b2e6-c1dadc81cfac" Version="2.2.90" Language="en-US" Publisher="Andreas Reich" />
<DisplayName>IncludeToolbox</DisplayName>
<Description xml:space="preserve">Various tools for managing C/C++ #includes: Formatting, sorting, exploring, pruning.</Description>
<License>license.txt</License>
Expand Down
60 changes: 49 additions & 11 deletions IncludeToolbox/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.TextManager.Interop;
using Microsoft.VisualStudio.VCProjectEngine;
//using Microsoft.VisualStudio.VCProjectEngine;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was this just left over from an old version? VS2017 complains about it.

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm odd. With the newest VS2017 version I'm hitting this as well. Sounds like I should make sure stuff works still in VS2015 before I upload to the marketplace.
Better remove it for good


namespace IncludeToolbox
{
Expand Down Expand Up @@ -35,23 +35,61 @@ public static string MakeRelative(string absoluteRoot, string absoluteTarget)
return relativePath;
}

public static string GetExactPathName(string pathName)
// Shamelessly stolen from https://stackoverflow.com/a/5076517/153079
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is similar to the previous functionality, but should be more reliable since each part of the path is normalized with GetFileSystemInfos. It's important to get this right, because eventually we rely on string comparisons to determine whether a file is in/under a directory path.

Copy link
Owner

Choose a reason for hiding this comment

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

Interesting. I'm not sure I could really follow the discussion on that SO page. Can you give me an example where GetExactPathName gives a different result than GetFileSystemCasing for absolute paths?
Would be really nice if we could use the same function for both relative and absolute paths and then use only that one. I really don't like having two file resolve functions around. So far I relied on getting deterministic cased absolute paths back from GetExactPathName

Copy link
Collaborator Author

@dakotahawkins dakotahawkins Sep 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Honestly, I didn't hit a problem-case, but I became very concerned about it because I was adding code that uses naked string operations to compare two different paths.

However, there aren't really two functions now, GetFileSystemCasing is private, and GetExactPathName calls it. I did it that way because the core work is recursive but there's also work you want to do before you start and after you finish, in this case normalizing the separators and adding one to the end of directories respectively.

Ignoring the possibility that the old GetExactPathName might not have normalized the case 100% of the time (again, not positive about that, just worried), the only difference now should be that directory paths will have a trailing separator. That's important because there are (and already were) places where string.StartsWith was used on two paths (prevents accidentally saying that C:\foo\bar.txt is inside of C:\foo\bar\).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reading back over the SO comments, I think the old version probably did work. The only thing I'm not 100% sure about is using DI Name instead of FullName. It probably worked fine, since in both cases the DI is built from GetFileSystemInfos(), but I also think the SO people arguing about how to do this did more pedantic/specific testing than you or I.

private static string GetFileSystemCasing(string path)
{
if (!File.Exists(pathName) && !Directory.Exists(pathName))
return pathName;
if (Path.IsPathRooted(path))
{
path = path.TrimEnd(Path.DirectorySeparatorChar); // if you type c:\foo\ instead of c:\foo
try
{
string name = Path.GetFileName(path);
if (name == "")
return path.ToUpper() + Path.DirectorySeparatorChar; // root reached

var di = new DirectoryInfo(pathName);
string parent = Path.GetDirectoryName(path);

if (di.Parent != null)
{
return Path.Combine(
GetExactPathName(di.Parent.FullName),
di.Parent.GetFileSystemInfos(di.Name)[0].Name);
parent = GetFileSystemCasing(parent);

DirectoryInfo diParent = new DirectoryInfo(parent);
FileSystemInfo[] fsiChildren = diParent.GetFileSystemInfos(name);
FileSystemInfo fsiChild = fsiChildren.First();
return fsiChild.FullName;
}
catch (Exception ex)
{
Output.Instance.ErrorMsg("Invalid path: '{0}'\nError:\n{1}", path, ex.Message);
}
}
else
{
return di.Name.ToUpper();
Output.Instance.ErrorMsg("Absolute path needed, not relative: '{0}'", path);
}

return "";
}

/// <summary>
/// Gets the path name as it exists it the file system, normalizing it.
/// </summary>
/// <param name="pathName">Path name</param>
/// <returns>Normalized path name. Directories are returned with trailing separator.</returns>
public static string GetExactPathName(string pathName)
{
if (!File.Exists(pathName) && !Directory.Exists(pathName))
{
return pathName;
}

string exactPathName = GetFileSystemCasing(Path.GetFullPath(pathName).Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar));
if (exactPathName == "")
return pathName;

// Add trailing slash for directories
exactPathName = exactPathName.TrimEnd(Path.DirectorySeparatorChar);
if (File.GetAttributes(exactPathName).HasFlag(FileAttributes.Directory))
return exactPathName + Path.DirectorySeparatorChar;
return exactPathName;
}

/// <summary>
Expand Down