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.

Sign up for GitHub

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

Jump to bottom

Make it work with windows, but also with posix #6

Closed
wants to merge 4 commits into from
Closed

Make it work with windows, but also with posix #6

wants to merge 4 commits into from

Conversation

cjblomqvist
Copy link

Fixes #1 in a real way that works with current version of browserify, without requiring any specific options passed (in contrast with #4 which actually breaks for me in the browser because process is not required and actually non-existing).

This should be pretty harmless to pull in and it would really really be nice to have it pulled in or some way to replace the currently built in path-browserify by some option somewhere (it seems only non-builtin dependencies can be swapped at the moment).

This was referenced Nov 13, 2015
Closed
Open
martinheidegger added a commit to potree/potree that referenced this pull request Sep 20, 2017
@martinheidegger
Fixed browserify issue in las-perf (note: related to browserify/path-…
30ed683 
…browserify#6)
martinheidegger added a commit to martinheidegger/potree that referenced this pull request Sep 22, 2017
@martinheidegger
Fixed browserify issue in las-perf (note: related to browserify/path-…
45ada62 
…browserify#6)
@stefanbuck stefanbuck mentioned this pull request Jan 3, 2018
Closed
@goto-bus-stop
Copy link
Member

Sorry that this sat there for so long. These days Node.js has a path.posix and a path.win32 API. I think it's important for the path-browserify module to expose a single predictable API by default, so that web apps don't behave differently depending on what system they were built on. The /-style paths are most useful in a browser since that is also what URLs use.

I'll close this now, but I am interested in adding path.win32 support to this module in the future. Meanwhile, thanks for the work, even if it didn't make it in!

@cjblomqvist
Copy link
Author

Thanks for acknowledging the PR, despite the long time. I still believe it needs to be addressed/handled, but I leave that up to you/others.

@Nantris
Copy link

Nantris commented Jan 15, 2022 •
edited
Loading

@goto-bus-stop I find it odd that there wouldn't be interest in supporting Windows.

From the readme:

If there is a difference in behaviour between Node.js's path module and this module, please open an issue!

I'd consider this to be a very crucial difference. This library not supporting Windows style path separators makes creating cross-platform code a lot harder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Windows support
3 participants
@cjblomqvist @goto-bus-stop @Nantris