[minor] Return the root itself from AbsoluteDirectoryPath.Parent - #144
Merged
Merged
Conversation
Walking up from a root reported an empty path. Empty passes AbsoluteDirectoryPath validation but is not a location: passed to the filesystem it reads as "no contents" rather than failing, so callers walking upwards silently landed nowhere instead of stopping at the root. A root is now its own parent, matching how the filesystem resolves /.. to / and C:\.. to C:\. Every value Parent returns is therefore a usable absolute directory. Use IsRoot, or compare the parent against the path, to detect that walking upwards has finished. GetAncestors already guarded against the self-referential root, so its results are unchanged. Surfaced by ktsu-dev/ImGuiApp#281, where the ".." row of the filesystem browser crashed on Linux: it trimmed separators off the path string, which also strips the leading separator that is the root on Unix, leaving a relative path that failed AbsoluteDirectoryPath validation. Claude-Session: https://claude.ai/code/session_01UwSLytSdW2pHmSmsSBAj8r
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




Problem
AbsoluteDirectoryPath.Parentreturned an empty path at a root. Empty passesAbsoluteDirectoryPathvalidation (it is allowed for initialisation), but it is not a location — passed to the filesystem it reads as "no contents" rather than failing, so callers walking upwards silently landed nowhere instead of stopping at the root.Surfaced by ktsu-dev/ImGuiApp#281, where the
..row of the ImGui filesystem browser crashed on Linux. That code trimmed separators off the path string, which also strips the leading separator — the root itself on Unix — leaving a relative path that failed validation:The consumer-side trimming is fixed separately; this PR removes the reason to hand-roll it at all.
Change
A root is now its own parent, matching how the filesystem resolves
/..to/andC:\..toC:\. Every valueParentreturns is therefore a usable absolute directory. UseIsRoot, or compare the parent against the path, to detect that walking upwards has finished.Scoped to
AbsoluteDirectoryPath.RelativeDirectoryPath.Parentand theDirectoryPathprimitive are untouched — "the parent ofa" is a genuinely different question for relative paths, and existing tests pin that behaviour.Compatibility
This changes documented behaviour of a public property, hence
[minor]. Anyone using the empty path as a "no parent" sentinel changes silently —IsRoothas always been the supported check. No new infinite-loop risk: empty was already a fixed point (Parentof""is""), sowhile (true) d = d.Parent;never terminated before either.GetAncestorsalready carriedif (next == current) break; // Prevent infinite loop at root, so its results are unchanged.Verification
Semantics.Test/Paths/AbsoluteDirectoryPathParentTests.cs(7 tests): parent chains, root-is-its-own-parent, UNC share roots,Parentnever returning an empty or relative path while walking up, termination at the fixed point, andGetAncestorsending at the root without repeating it. Uses the repo's per-OS ternary convention so the Unix cases execute on Linux rather than skipping.ktsu.Semantics.Paths.dllon real Linux (WSL):/home/matt/Documents → /home/matt → /home → / → /,IsRoot("/")true,GetAncestorscorrect.🤖 Generated with Claude Code