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

Add os.pathMatches proc #7836

Closed
wants to merge 8 commits into from
Closed

Add os.pathMatches proc #7836

wants to merge 8 commits into from

Conversation

data-man
Copy link
Contributor

I don't have Windows installed, so ...

@data-man
Copy link
Contributor Author

Maybe it's need to add to the description that the pattern is OS specific.

@Araq
Copy link
Member

Araq commented May 18, 2018

Nice but os.nim is getting bloated, put it somewhere else (osutils.nim?).

@data-man
Copy link
Contributor Author

To new module? What else will be in this module?
Maybe to ospaths? (but it's anyway imported in os)

@data-man
Copy link
Contributor Author

@Araq

Let's merge it, as is?

@Varriount
Copy link
Contributor

PathMatchSpec isn't equivalent to fnmatch. Additionally, the function lacks any documentation or examples detailing exactly what "pattern" should be.

@data-man
Copy link
Contributor Author

@Varriount

PathMatchSpec isn't equivalent to fnmatch.

Why isn't? See tests.

function lacks any documentation or examples detailing exactly what "pattern" should be.

I proposed to add:

Maybe it's need to add to the description that the pattern is OS specific.

No comments.

@Araq
Copy link
Member

Araq commented May 22, 2018

Add it as an helper to where you need it please. No further bloat of os.nim. Nobody asked for this feature.

@data-man
Copy link
Contributor Author

Nobody asked for this feature.

Isn't true.
https://github.com/citycide/glob

@data-man data-man closed this May 22, 2018
@data-man
Copy link
Contributor Author

It's useful.

@data-man
Copy link
Contributor Author

data-man commented May 22, 2018

It's useless.

@data-man data-man reopened this May 22, 2018
@dom96
Copy link
Contributor

dom96 commented May 22, 2018

Sorry @data-man but I agree with @Araq. Create a Nimble package for this.

@data-man data-man closed this May 22, 2018
@data-man
Copy link
Contributor Author

Well, closed.
I planned use this for other stdlib's PRs and compiler.

@Araq
Copy link
Member

Araq commented May 22, 2018

I planned use this for other stdlib's PRs and compiler.

You don't have to add it to the stdlib in order to use it...

@timotheecour timotheecour added the OS (stdlib) Related to OS modules in standard library label Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS (stdlib) Related to OS modules in standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants