root search pattern for hashfiles #151
Conversation
|
lgtm |
| @@ -28,9 +29,26 @@ internal sealed class HashFiles : Function | |||
| string searchRoot = workspaceData.Value; | |||
| string pattern = Parameters[0].Evaluate(context).ConvertToString(); | |||
|
|
|||
| // Convert slashes on Windows | |||
There was a problem hiding this comment.
I've updated the PR. Simply rooting the path caused issues on Windows.
Now i've also added code to treat \ and / equivalent on Windows. This is consistent with the updated ADR.
| var files = Directory.GetFiles(searchRoot, "*", SearchOption.AllDirectories).OrderBy(x => x).ToList(); | ||
| var files = Directory.GetFiles(searchRoot, "*", SearchOption.AllDirectories) | ||
| .Select(x => s_isWindows ? x.Replace('\\', '/') : x) | ||
| .OrderBy(x => x, StringComparer.Ordinal) |
There was a problem hiding this comment.
I spent time thinking about the default comparer, current culture vs ordinal. Even though i eventually realized the default is ordinal here (iirc), i went ahead and specified ordinal explicitly so it's now obvious to the reader that the author thought about it.
| if (files.Count == 0) | ||
| { | ||
| throw new ArgumentException($"'hashFiles({pattern})' failed. Directory '{searchRoot}' is empty"); | ||
| throw new ArgumentException($"hashFiles('{ExpressionUtility.StringEscape(pattern)}') failed. Directory '{searchRoot}' is empty"); |
There was a problem hiding this comment.
better error message so the syntax doesnt look malformed
| @@ -83,12 +106,14 @@ internal sealed class HashFiles : Function | |||
| } | |||
| } | |||
|
|
|||
| private static readonly bool s_isWindows = Environment.OSVersion.Platform != PlatformID.Unix && Environment.OSVersion.Platform != PlatformID.MacOSX; | |||
There was a problem hiding this comment.
used in multiple places now, so moved to private field
|
We can/should cherry-pick this directly into |
fixes issue #146 and #154