-
Notifications
You must be signed in to change notification settings - Fork 146
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
Improve folder detection in _info #313
base: main
Are you sure you want to change the base?
Conversation
What happens if you both have the empty placeholder and nested directories? This should be tested for the cases that:
With this change, is is still true that |
You mean in case there is I'll add some test cases at the end of the week, can't answer these out of my head. At the moment (with or without this PR) the behaviour seems to be incorrect as |
I suppose both those problematic cases :|
If you do with |
@dynamix @martindurant Any updates on the PR? The inconsistency is causing issues for us while working on remote filesystems. Ref |
I would like some additional tests for each of the cases I listed above. It's not obvious to me from the code alone that every case will pass. |
out0 = [o for o in out if o["name"].rstrip("/") == path] | ||
if out0: | ||
out0 = next((o for o in out if o["name"].rstrip("/") == path), None) | ||
if out0 and out0["name"][-1] != "/" and out0["size"] == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if out0 and out0["name"][-1] != "/" and out0["size"] == 0: | |
if out0 and (out0["name"][-1] != "/" or out0["size"] == 0): |
A suggestion to fix #312.