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

forward-slashed ids for Windows #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

forward-slashed ids for Windows #118

wants to merge 1 commit into from

Conversation

ijmo
Copy link

@ijmo ijmo commented Jan 27, 2016

Forward-slash-separated ids are supported on Windows.

e.g.
:ids #{"js/main"} instead of :ids #{"js\main"}

@Deraen
Copy link
Contributor

Deraen commented Jan 29, 2016

Have you checked if boot-reload and boot-cljs-repl have the same problem?

They were implemented after boot-cljs so they are using by-path but will just build path by concatenating the extension to it, i.e. with forward slashes:

https://github.com/adzerk-oss/boot-reload/blob/master/src/adzerk/boot_reload.clj#L70-L75

Would be super if you can check those and send PR to boot-reload and boot-cljs-repl if needed.

@ijmo
Copy link
Author

ijmo commented Jan 29, 2016

I saw it just now. Those seem to have same issue on Windows. Let me make PRs to both boot-reload and boot-cljs-repl. Happy to help!

@Deraen
Copy link
Contributor

Deraen commented Jan 29, 2016

If by-path has always problems in Windows, I wonder if this should be fixed in Boot itself instead of all libraries using it.

@ijmo
Copy link
Author

ijmo commented Jan 29, 2016

I think you're right. Seeing by-path, it does not handle the separator problem. It should be fixed not boot-reload or boot-cljs-repl.

@micha
Copy link
Contributor

micha commented Jan 29, 2016

@Deraen yeah, i think boot should only use forward slashes, but that will be a breaking change. I think it's something that will simplify things for windows users, though, so perhaps making the breaking change now is the best approach (it's only a breaking change for windows users, who are having to deal with all these issues anyway).

One of the biggest issues with backslash paths is the discrepancy between filesystem paths and URIs, since URIs are not platform-specific and use only forward slashes. Since the classpath is a collection of URIs this becomes a pretty big pain when we're dealing with files that are on the classpath—most of what boot and boot tasks do.

I think we should start to think about how to fix boot and make the change as soon as we're confident we have a good solution.

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

Successfully merging this pull request may close these issues.

3 participants