Skip to content
This repository has been archived by the owner on Mar 23, 2018. It is now read-only.

Fix cross-platform issues with getFiles() #25

Merged
merged 2 commits into from
Jan 4, 2017

Conversation

foxbunny
Copy link
Contributor

@foxbunny foxbunny commented Jan 3, 2017

It was discovered that getFiles() does not handle Windows paths very well. This
patch addresses the issue by:

  1. Ensuring the tests are cross-platform themselves
  2. All input and output paths are properly normalized by using
    path.normalize() and path.join().

Issue originally reported in #24

NOTE: I haven't had a chance to test this on a non-Windows computer yet.

Signed-off-by: Hajime Yamasaki Vukelic hayavuk@gmail.com

Hajime Yamasaki Vukelic added 2 commits January 3, 2017 12:40
It was discovered that getFiles() does not handle Windows paths very well. This
patch addresses the issue by:

1. Ensuring the tests are cross-platform themselves
2. All input and output paths are properly normalized by using
  `path.normalize()` and `path.join()`.

Signed-off-by: Hajime Yamasaki Vukelic <hayavuk@gmail.com>
Signed-off-by: Hajime Yamasaki Vukelic <hayavuk@gmail.com>
@gpbl gpbl merged commit 22d675c into gpbl:master Jan 4, 2017
@gpbl
Copy link
Owner

gpbl commented Jan 4, 2017

Awesome thanks! I can't publish the fix on npm right now as I'm traveling - I'll do it ASAP :)

@ghost
Copy link

ghost commented Feb 10, 2017

@gpbl I'm in the process of adopting webpack-cleanup-plugin, but have encountered the same issues as @foxbunny. I'm not sure my path issues are entirely resolved by this change, but I'd like to try it out as published, before attempting to debug it further.

@gpbl
Copy link
Owner

gpbl commented Feb 11, 2017

I'm just back from my trip – going to publish this soon! Thanks for the patience!

@foxbunny
Copy link
Contributor Author

@destroyerofbuilds Once this patch is published (if it isn't already), please try it and file a new issue if your issue persists. Mention me in the issue and I'll try to respond and fix it.

@gpbl
Copy link
Owner

gpbl commented Feb 12, 2017

Thanks @foxbunny for the help! Yes it is already published 👍🏽

@ghost ghost mentioned this pull request Feb 13, 2017
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants