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

includeHidden=false doesn't prevent recursion into hidden directories #7

Closed
timotheecour opened this issue Jul 5, 2018 · 6 comments
Labels
external Issues with dependencies or otherwise unsolvable within the project itself. good first issue Probably good for a user's first contribution. type: bug Something isn't working as intended or expected.

Comments

@timotheecour
Copy link
Collaborator

git clone https://github.com/AndreiRegiani/INim
cd INim

nim c -o:app -r /tmp/t31_glob.nim

/tmp/t31_glob.nim:

import glob
import strutils
let m = listGlob(".", includeHidden=false)
echo m.join("\n")

prints lines such as:

./src/nimcache/stdlib_linenoise.c
./.git/config
./.git/HEAD

which should've been filtered out by includeHidden=false

perhaps you're only filtering out hidden files, not hidden folders?

@haltcase
Copy link
Owner

haltcase commented Jul 6, 2018

You're right, the source is here: https://github.com/citycide/glob/blob/ca0513dca6896972bf5277b4ec94202f51ba49a9/src/glob.nim#L314

The folder itself isn't yielded but when recursing there isn't another check before adding that directory to the stack. Should be a simple fix (~isRec and (includeHidden or not path.isHidden) ) if you'd like to try a PR.

@haltcase haltcase added help wanted Extra attention is needed status: accepted Change is accepted and is open to community PRs. type: bug Something isn't working as intended or expected. good first issue Probably good for a user's first contribution. labels Jul 6, 2018
@timotheecour
Copy link
Collaborator Author

unfortunately my local fix hit that bug: nim-lang/Nim#8225

@haltcase
Copy link
Owner

haltcase commented Jul 6, 2018

Thanks for the update. This is really the only untested thing right now as I wasn't sure how to go about testing for hidden files. Should get some tests in there once we get this figured out.

timotheecour added a commit to timotheecour/glob that referenced this issue Jul 7, 2018
haltcase pushed a commit that referenced this issue Jul 7, 2018
@haltcase haltcase changed the title includeHidden=false doesn't work includeHidden=false doesn't prevent recursion into hidden directories Jul 7, 2018
@haltcase
Copy link
Owner

haltcase commented Jul 7, 2018

Fix is pushed on our end, just need nim-lang/Nim#8315 to be merged and released.

@haltcase haltcase added external Issues with dependencies or otherwise unsolvable within the project itself. status: pending release Issue is resolved but waiting to be released. and removed status: accepted Change is accepted and is open to community PRs. labels Jul 7, 2018
@haltcase haltcase removed the help wanted Extra attention is needed label Jul 15, 2018
@haltcase
Copy link
Owner

Closing since we've fixed this in glob. If anyone still has this issue, update Nim itself to a version that includes the mentioned PR.

@haltcase haltcase removed the status: pending release Issue is resolved but waiting to be released. label Sep 26, 2018
@timotheecour
Copy link
Collaborator Author

nim-lang/Nim#8315 was finally merged, fixing this for good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Issues with dependencies or otherwise unsolvable within the project itself. good first issue Probably good for a user's first contribution. type: bug Something isn't working as intended or expected.
Projects
None yet
Development

No branches or pull requests

2 participants