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

Windows compatibility #158

Merged
merged 11 commits into from Jun 5, 2012
Merged

Windows compatibility #158

merged 11 commits into from Jun 5, 2012

Conversation

@domenic
Copy link
Contributor

@domenic domenic commented Jun 5, 2012

This supersedes #113, #125, #126, #137, and #144.

82/83 tests pass. test/watch.js is the failure, with "Cannot find module './a'" I am not sure what this is about.

Please consider merging. I am happy to make any changes you desire to make it more merge-able.

domenic added 11 commits Jun 5, 2012
Before it was saying `EventEmitter.require` in the stack traces; now it says `Wrap.require`.
On Windows paths have backslashes, instead of forward slashes.
Previous substitutions had made them always IDs. If it's an absolute path, use `path.normalize` to revert them back to paths.
The existing logic was turning '/node_modules/jade/lib/jade.js' into 'C:\node_modules\jade\lib\jade.js', when '/node_modules/jade/lib/jade.js' was desired.

Fixes Jade tests.
.js files aren't valid Win32 applications. It should spawn `node`, with the .js file as an argument.
Have to use `fs.watch` instead of `fs.watchFile`. Tests still do not pass due to "Cannot find module './a'". So not sure if this was a good idea.
@substack substack merged commit b739766 into browserify:master Jun 5, 2012
@substack
Copy link
Collaborator

@substack substack commented Jun 5, 2012

The failing watch test was just from treating fs.exists || path.exists like existsSync. All fixed and thanks for getting this running on windows!

@jdahlq
Copy link

@jdahlq jdahlq commented Jun 18, 2012

Thanks for all the hard work, @domenic!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants