Skip to content

fix: treefmt . no longer segfaults if . is root and symlink #595

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

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

Conversation

jfly
Copy link
Collaborator

@jfly jfly commented May 25, 2025

In short, the old code would massage the given paths to relative paths (in the resolvePaths function I removed). It operated under an impossible-to-satisfy requirement: to return relative paths that when joined with the project root would create paths to regular files/folders (not symlinks). This is possible for all paths except .: if your project root is a symlink, then join(root, ".") == root, which is still a symlink.

Rather than try to fix that flawed assumption, I opted to refactor things. I think that was the right call: excluding the tests I added, this is a net reduction in code!

Basically, I moved moved resolvePath into walk.go, and interleaved the remaining resolvePaths logic into walk.go::NewCompositeReader. A happy side effect of this is that we no longer have 2 functions with very similar names (resolvePaths and resolvePath) that do different things.

This fixes #594

In short, the old code would massage the given paths to relative paths
(in the `resolvePaths` function I removed). It operated under an
impossible-to-satisfy requirement: to return relative paths that when
joined with the project root would create paths to regular files/folders
(*not* symlinks). This is possible for all paths except `.`: if your
project root is a symlink, then `join(root, ".") == root`, which is
still a symlink.

Rather than try to fix that flawed assumption, I opted to refactor
things. I think that was the right call: excluding the tests I added,
this is a net reduction in code!

Basically, I moved moved `resolvePath` into `walk.go`, and interleaved
the remaining `resolvePaths` logic into `walk.go::NewCompositeReader`.
A happy side effect of this is that we no longer have 2 functions with very similar names
(`resolvePaths` and `resolvePath`) that do different things.

This fixes numtide#594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

treefmt . at the root of the project fails if the project root is a symlink
1 participant