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

commands/set-tree: checks if on windows, if so use \ instead of / #48

Merged
merged 1 commit into from
Apr 9, 2022

Conversation

saikyun
Copy link
Contributor

@saikyun saikyun commented Apr 5, 2022

On windows using jpm -l deps I got errors about "Invalid switch /i". After investigation I found that set-tree didn't take windows into account, so I amended that. Now I am able to use jpm -l. :)

@sogaiu
Copy link

sogaiu commented Apr 5, 2022

May be related a bit to #45.

@bakpakin
Copy link
Member

bakpakin commented Apr 6, 2022

So one reason I haven't been bullish on this is that modern windows should be able to use "/" in most (all?) scenarios - with the exception of shelling out to certain paths. So please let me know if this is is actually broken or "it just looks more windowsy".

@sogaiu
Copy link

sogaiu commented Apr 6, 2022

Before #45 was merged I experienced breakage (running jpm -l deps in the cross subdirectory of https://github.com/saikyun/jaylib-wasm-demo (around 83eca893ff73fb9e54a8d9d744392927157a3cc9)).

I just tried with a version of jpm that had #45 merged and I don't see the same problems any more.

May be @saikyun can confirm.

@saikyun
Copy link
Contributor Author

saikyun commented Apr 7, 2022

I had the same problem as sogaiu.
For some time I've been seeing errors like "Invalid switch /i" when building, but it hasn't hindered building. However when using jpm -l and (janet-bounded-queue)[https://github.com/saikyun/janet-bounded-queue], the builds would fail (it couldn't find janet-bounded-queue). Only thing I can see is different about janet-bounded-queue is that it's a single file.

After using Janet built with this PR all errors disappeared.

@saikyun
Copy link
Contributor Author

saikyun commented Apr 8, 2022

I didn't realize sogaiu mentioned a different PR, gonna have to try that one.

@bakpakin bakpakin merged commit dacd2bb into janet-lang:master Apr 9, 2022
@saikyun
Copy link
Contributor Author

saikyun commented Apr 11, 2022 via email

@saikyun
Copy link
Contributor Author

saikyun commented Oct 11, 2022 via email

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