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

PowerShell scripts for lfc and lff #1430

Merged
merged 5 commits into from
Oct 28, 2022
Merged

Conversation

axmmisaka
Copy link
Collaborator

As mentioned in the commit message, this current implementation is very *NIX-y/bashy and not powershell-y. While I don‘t doubt its correctness this adds complications; maybe we should just pass the thing in as argument.

Right now, launch.ps1 will, just like how lfc and lff (which are symbolic links) on *NIX works, get the name of the invoker and run the corresponding jar file, which is not as PowerShell-y. We can use arguments like outlined here: https://stackoverflow.com/a/43940404.
Nevertheless, it appears it does work somehow.
@axmmisaka
Copy link
Collaborator Author

There appears to be some misunderstanding until I saw this: #1422 (comment)

Copy link
Collaborator

@cmnrd cmnrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! While testing this I noticed one problem though. It seems like we cannot pass any additional arguments to lfc and lff anymore. For instance .\bin\lfc.ps1 -c test\C\src\Minimal.lf produces an error complaining that test\C\src\Minimal.lf does not exist. Do you have an idea what is going on here? It works perfectly fine without any additional arguments.

@axmmisaka
Copy link
Collaborator Author

axmmisaka commented Oct 27, 2022

Ah I see, my hypothesis is that PS takes ./bin as its PWD… because running .\bin\lfc.ps1 -c .\..\test\C\src\Minimal.lfworks.

Ah I see, powershell parsing error…… I always forget that powershell pass arguments as “objects” not strings!

@axmmisaka
Copy link
Collaborator Author

Unsure if using . or Invoke-Command would be better

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for fixing this :-)

@lhstrh lhstrh merged commit afcb7e6 into refactor-cli Oct 28, 2022
@cmnrd cmnrd deleted the axmmisaka/refactor-cli-windows branch October 28, 2022 08:14
@cmnrd
Copy link
Collaborator

cmnrd commented Oct 28, 2022

Thanks @axmmisaka!

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