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

nsist: use "/" when comparing fnmatch patterns #148

Merged
merged 1 commit into from Mar 27, 2018

Conversation

Projects
None yet
2 participants
@davvid
Contributor

davvid commented Mar 24, 2018

Canonicalize paths to their Unix representation when comparing fnmatch
patterns. This makes patterns work across platforms, and
simplifies usage by providing a consistent interface to the
exclude patterns.

Closes #39

@davvid

This comment has been minimized.

Contributor

davvid commented Mar 24, 2018

Note: this is based off of the same branch as #147 so please merge that one first.

@takluyver

This comment has been minimized.

Owner

takluyver commented Mar 26, 2018

Thanks! I like the idea of making it platform independent, but let's normalise the exclude patterns as well - that way the change shouldn't break things for anyone with Windows style paths in their exclude patterns.

Also, it's probably a good idea to note in the docs that the patterns can use either kind of slash, and it will be normalised to /.

nsist: normalize paths when applying exclude patterns
Canonicalize paths to their Unix representation when applying
exclude patterns.  This makes patterns work across platforms, and
simplifies usage by providing a consistent interface to the
exclude patterns.

Closes #39

@davvid davvid force-pushed the davvid:canonical-paths branch from 76a0425 to 69297e0 Mar 27, 2018

@davvid

This comment has been minimized.

Contributor

davvid commented Mar 27, 2018

Good idea. I've updated this branch so that exclude patterns are normalized too, and added a note to the docs about the patterns being cross-platform.

@@ -276,6 +276,10 @@ the line with the key:
contradicts your ``files`` or ``packages`` option, the files in question
will be included (you can not exclude a full package/extra directory
or a single file listed in ``files``).
* Exclude patterns are applied uniformly across platforms and can use
either Unix-style forward-slash (`/`), or Windows-style back-slash (`\`)

This comment has been minimized.

@takluyver

takluyver Mar 27, 2018

Owner

N.B. inline code in rst uses double backticks. It's possible to configure single backticks to mean the same, but I don't think I have on this project.

We should also build the docs and check whether the backslash shows up - it might be treated as an escape.

@takluyver

This comment has been minimized.

Owner

takluyver commented Mar 27, 2018

I'll merge this and fiddle with the docs myself - thanks!

@takluyver takluyver merged commit d635e4b into takluyver:master Mar 27, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@davvid davvid deleted the davvid:canonical-paths branch Mar 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment