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

[critical] walkGlob doesn't work with un-normalized paths (eg containing //); os.walkFiles works #35

Closed
timotheecour opened this issue Jan 16, 2020 · 0 comments · Fixed by #37
Labels
help wanted Extra attention is needed type: bug Something isn't working as intended or expected.

Comments

@timotheecour
Copy link
Collaborator

timotheecour commented Jan 16, 2020

the // in input pattern makes walkGlob not work; it should work, like other tools (eg: unix ls on cmdline, or os.walkFiles)

  # D20200115T205019
  import pkg/glob
  import std/os
  proc main()=
    let pattern2 = "/Users/timothee/temp//d18_D20200115T203454/*.log"
    echo "walkGlob:"
    for file in walkGlob(pattern2):
      echo (file: file)
    echo "walkFiles:"
    for file2 in walkFiles(pattern2):
      echo (file2: file2)
  main()

output:

walkGlob:
walkFiles:
(file2: "/Users/timothee/temp//d18_D20200115T203454/foo1.foo2.foo3.log")
(file2: "/Users/timothee/temp//d18_D20200115T203454/foo1.foo2.log")
(file2: "/Users/timothee/temp//d18_D20200115T203454/foo1.log")

note

double slashes can and do occur in practice, eg if user has an env var MYDIR=/path/to/foo/ and enters a path like $MYDIR/foo.txt ;
this is not uncommon practice, because ending dirs in / (eg MYDIR=/path/to/foo/ ) makes sense, and concatenating with / (eg $MYDIR/foo.txt) also makes sense in case user is not sure whether $MYDIR ended in a /
I can point to more stackoverflow issues pointing to how this does happen in practice, if needed

proposal

walkGlob shd probably simply call normalizedPath on the input pattern before further processing

@haltcase haltcase added help wanted Extra attention is needed type: bug Something isn't working as intended or expected. labels Jan 16, 2020
timotheecour added a commit to timotheecour/glob that referenced this issue Jan 17, 2020
haltcase pushed a commit that referenced this issue Jan 17, 2020
* fix #35 ; simplify code a bit
timotheecour added a commit to timotheecour/glob that referenced this issue Jan 19, 2020
haltcase pushed a commit that referenced this issue Jan 20, 2020
Closes #35

* re-attempt "fix #35; simplify code a bit"

This reverts commit 84dd5dd.

* fix for windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed type: bug Something isn't working as intended or expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants