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

[RFC] glob needs a way to do error reporting (eg unaccessible files) #39

Open
timotheecour opened this issue Jan 18, 2020 · 3 comments
Open
Labels
discussion [RFC] Fixes, APIs, or changes need feedback. type: feature Request for a new feature or enhancement.

Comments

@timotheecour
Copy link
Collaborator

this is related to nim-lang/Nim#13163

currently, walkGlob won't report any errors when trying to recurse inside dirs with invalid permissions.
there should be a way to report such errors; exactly how needs to be discussed;
eg:

  • via a callback(we already have callbacks in api, filterDescend, and filterYield)
  • (preferred IMO) by adding an additional field in GlobEntry, eg a ErrorKind
type ErrorKind=enum
  none,
  invalidPermission,
type
  GlobEntry* =
    tuple[path: string, kind: PathComponent, error: ErrorKind]

question is whether this would break any code

@haltcase haltcase added discussion [RFC] Fixes, APIs, or changes need feedback. type: feature Request for a new feature or enhancement. labels Jan 18, 2020
@haltcase
Copy link
Owner

@timotheecour I would also prefer option 2, but it is a breaking change. Any code using walkGlobKinds will break if it's using tuple destructuring:

# will fail when `GlobEntry` is a 3-tuple
for path, kind in walkGlobKinds():
  discard

# will work
for path, kind, error in walkGlobKinds():
  discard

I'm ok with that given we document the change properly and we're still < v1, although I wonder if we should change GlobEntry to be an object at that point in case anything else needs to be added. And maybe rename walkGlobKinds to something else (walkGlobEntries?).

@timotheecour
Copy link
Collaborator Author

  • object seems better than tuple (for the exact reason you mentioned + other reasons)
  • instead of breaking code, how about deprecating walkGlobKinds (via {.deprecated: "use walkGlobEntries".}) and adding walkGlobEntries ?

@haltcase
Copy link
Owner

Well, interestingly enough walkGlobKinds is technically the base implementation and walkGlob is just a wrapper that drops the kind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion [RFC] Fixes, APIs, or changes need feedback. type: feature Request for a new feature or enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants