Skip to content
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

fix: also check if path is in project for --stdin #440

Closed
wants to merge 1 commit into from
Closed

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Oct 12, 2024

--stdin should operate under the same rules as the other walkers.

--stdin should operate under the same rules as the other walkers.
@brianmcgee
Copy link
Member

I remember now why it worked the way it did before. The stdin path doesn't exist yet. It will be created by the stdin walker when Read is called.

@brianmcgee brianmcgee closed this Oct 12, 2024
jfly added a commit to jfly/treefmt that referenced this pull request Oct 12, 2024
See numtide#440 for the last attempt to
do this because (at the time) this "is the file in the tree root?" check
relied upon checking for concrete files in the filesystem. That's no
longer the case, so we can make this code a bit more consistent.
jfly added a commit to jfly/treefmt that referenced this pull request Oct 12, 2024
See numtide#440 for the last attempt to
do this because (at the time) this "is the file in the tree root?" check
relied upon checking for concrete files in the filesystem. That's no
longer the case, so we can make this code a bit more consistent.
@jfly
Copy link
Collaborator

jfly commented Oct 12, 2024

The stdin path doesn't exist yet. It will be created by the stdin walker when Read is called.

Hmm... are you sure the stdin walker creates the file? That seems like odd behavior to me, and doesn't seem to be what's happening with the code on main.

#445 removes the requirement that the paths actually exist in order to check that they're under the tree root. Assuming that PR looks good, I've re-implemented @zimbatm's work here over in #446. I don't think it has any issues with missing files.

jfly added a commit to jfly/treefmt that referenced this pull request Oct 12, 2024
See numtide#440 for the last attempt to
do this because (at the time) this "is the file in the tree root?" check
relied upon checking for concrete files in the filesystem. That's no
longer the case, so we can make this code a bit more consistent.
jfly added a commit to jfly/treefmt that referenced this pull request Oct 12, 2024
See numtide#440 for the last attempt to
do this because (at the time) this "is the file in the tree root?" check
relied upon checking for concrete files in the filesystem. As of
numtide@ff3de21,
that's longer the case, so we can make this code a bit more consistent.
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.

3 participants