-
Notifications
You must be signed in to change notification settings - Fork 147
script extension can be either '.sc' or empty string #3802
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
Conversation
suggested changes Co-authored-by: Piotr Chabelski <[email protected]>
Co-authored-by: Piotr Chabelski <[email protected]>
…scala-cli into tune-script-definition
…scala-cli into tune-script-definition
@Gedochao - At least some of these test failures are download problems:
All but 1 of the 78 failures are passing on my Linux box. Reading the first 2 bytes of a script file to compare against Let me know if I should squash everything to a single commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'll squash the commits when merging, no worries.
modules/integration/src/test/scala/scala/cli/integration/RunScriptTestDefinitions.scala
Outdated
Show resolved
Hide resolved
…riptTestDefinitions.scala Co-authored-by: Piotr Chabelski <[email protected]>
The 2 failing tests pass on my
|
Suspecting intermittent failures on the CI, irrelevant to this PR... |
File with no extension only considered a script if file has a shebang header.
This is a follow-up to #3794 which passed CI tests but implemented a less restrictive script definition.
A script with a valid shebang header and no extension is recognized as a valid script even when launched from a shell that isn't shebang capable. Consequently, it can be run when directly passed to
scala-cli
without theshebang
command, just like a script with '.sc' extension.A side-effect of the less restrictive script definition in #3794 is that the wrapper class for a script with
.scala
extension also has adef scriptPath
defined, which is nice. However, without knowing all the potential side-effects of that change, it seems safer to restrict the scope here.