Improve compiler single-file run syntax

[RX14]

Now the compiler passes all arguments after the first filename to the program.

This allows crystal to be used as #!/usr/bin/env crystal and have arguments work correctly.

[oprypin]

I think referencing bin/crystal explicitly should be avoided if at all possible.
This would immediately fail on Windows, of course.

Maybe there should be a helper like this

crystal/spec/spec_helper.cr

Line 289 in 127ea04

def build_and_run(code)

but I suppose this is explicitly about running Crystal directly, so maybe that one doesn't apply.

Also this is really bad in terms of shell quoting. Please use Process.run with explicit args.

Also needs File.join.

[oprypin]

Does it distinguish flags that have a separate value?

E.g. would it understand --exclude-warnings foo.cr bar.cr?

Maybe worth a test.

[waj]

Does this mean executing like this doesn't work anymore?

$ crystal foo.cr --release

No big deal although I use it many times when I run something, press "up" to recall and add parameters. Now they have to be added before the file name. But this is actually a breaking change for any script out there.

[j8r]

Resolves #6291

[RX14]

Does this mean executing like this doesn't work anymore?

Yes, this changed. Ideally crystal --release foo.cr would work, but that's not possible currently with the way crystal handles options.

crystal run foo.cr --release still works.

[asterite]

In my mind #!/usr/bin/env crystal makes no sense because it will recompile the program every time.

What I do with my scripts: I compile them once, in release, and put them in /usr/local/bin. Then they run extremely fast.

Of course others don't need to do this... but I guess people are using Crystal as a scripting language, which boots fast, which is not.

[j8r]

It is ok for few cases. For example, CI scripts would need to be recompiled each time anyway.

We don't always need compile time speed matching scripting languages (some compiles to bytecodes before running).

[asterite]

some compiles to bytecodes before running

compiling to bytecode is miles faster than type checking

By the way, I use flags at the end all of the time. It's just so convenient to add or remove flags from the end than to go right after the build or run command and modify them there.

Can we not introduce a crystal shell_wrapper or something like that that works in this way, but leave the current way as is?

[oprypin]

If you want flags at the end, you can use crystal run with that.

[straight-shoota]

Just for reference: There's also https://github.com/Val/crun for shebang integration. Also does some cool things like caching the compiled binary IIUC.

I'm not sure whether this change is worth it. I can see the general use case for shebang, but is it really so useful to pass arguments? I use #!/usr/bin/env crystal a lot for one-off programs and it works fine if you don't need CLI arguments. And I feel when youthe program needs arguments, the intention is to re-use it which would greatly benefit from building en executable once instead of compiling on every run.
Against this questionable usefulnes there is the downside of restricting the way compiler arguments can be used. And IMO the latter is very commonly used and the fact that people are used to that pattern would make it hard if it breaks because you have to switch your own behaviour. Arguments with shebang programs have never worked. Making it work would be great, but it's IMO not a huge loss if it doesn't.

[waj]

Oh.. I wasn't aware of crun. I was going to suggest something similar. Erlang also has a escript command to compile and run in a single step: https://erlang.org/doc/man/escript.html

[asterite]

I retract my comment above. If it's useful for some then I see no problem.

That said, for example in go if you pass arguments after the filename to build you get an error saying that the options must come before the filename. We should probably do the same.

[RX14]

Most of the time, for a little script, you don't need compiler arguments. If you do need them when running from the commandline, then you can just add run to the command.

I think this is the best semantics, and allowing you to do crystal --release foo.cr args eventually is the intention. But to get to there, this breaking change needs to happen.

[j8r]

I was always confused with unexpected semantics with crystal [file] compared to crystal run [file], which is properly documented with crystal run --help. I always use the latter now.

crystal --help returns crystal [command] [switches] [program file] [--] [arguments], crystal file.cr -- --release should work then.

BTW I was never a fan of crystal myfile.cr, adding the 4 characters run was never a big deal for me 😄

[RX14]

Just rebased this and fixed the spec failure.

Would be neat if this could be in 0.35.0, I think the semantics are an improvement, even if they're not perfect.

[RX14]

Could we get this in for 0.35.0?

[waj]

I'm ok with this change, but why is the behaviour different from crystal run? crystal --help says run is the default command. Doesn't that mean that doing crystal foo.cr and crystal run foo.cr should be exactly the same? It's not the case already and this makes the difference even larger. Is there a good reason to keep this difference? If we want to encourage using Crystal as a script engine, at least we should add some caching like crun. Is there any reason not to add those semantics to current run command? Should we have a script command instead and make that the default? Too many questions? :)

[RX14]

Should we have a script command instead and make that the default? Too many questions? :)

I like this, it might help clarify the semantics! But, is this required for merging this PR?

Adding script caching is something I certainly considered too. I think the fact that crun exists means that people want this functionality, but having it as a separate executable you have to install makes it impractical to use. So I thought the compiler could do some of what crun does.

[waj]

is this required for merging this PR?

That's what I was wondering. Because this PR might promote using just crystal and that would force us to keep that behaviour to not break compatibility with existing scripts in the future. So, for example if we later decide to have both run and script, what's the default? If we keep run, then scripts should be written with crystal script then. So, to answer your question, I don't know yet. This seems simple, but it requires some more analysis.

[RX14]

It's pretty clear that it'd be script as the default if we add it, to not break things. It's just a question of whether we want these semantics (which you seem OK with), and whether it's possible to keep those semantics moving forward (with script it seems to be).

Using #!/usr/bin/env crystal script will not work, because linux passes only a single argument to the program, so env will simply try and find /usr/bin/crystal script, and fail. Yep, it's stupid, but this is why no other language uses subcommands: it doesn't work, and linux can't change this behaviour at this point for compatibility reasons.

[RX14]

Do we count compiler commandline behaviour as part of the 1.0 guarantee? So can this be merged after 1.0 or not?

[jkthorne]

@RX14 that is a good question as a user of crystal I would hope the CLI interface was consistent between versions.

[waj]

@RX14 I think we should make everything as a guarantee after 1.0 as much as possible. So, command line parameters should be forward compatible, at least until 2.0. To answer your question, we should merge this before 1.0.

So, to clarify, after this PR is merged:

• crystal foo.cr arg will pass arg as argument (will never try to understand it as compiler parameter)
• crystal run foo.cr arg will parse arg as compiler parameter (will never pass as argument)

I would agree with that behaviour. Could you please open an issue to create crystal script later and adjust --help output to clarify that the default is an alias of script and not run?

[bcardiff]

Maybe we need a spec to ensure the single file mode allows compiler flags before the file, right?

$ crystal -Dflag foo.cr arg

I understand that that flag won't be able to be used in the shebang. Should we discuss how the compiler flags can be passed in that scenario?

Other than that, I agree with what @waj said.

[asterite]

Maybe this is not known, but you can do:

crystal foo.cr bar.cr

that will consider both files for compilation and produce a single executable.

Does this PR change that behavior? I'm not saying that it's a good idea to keep that (I only use that when wanting to run multiple specs, but doing crystal spec file1.cr file1.cr works and is better because I can specify line numbers too). I'm just asking whether the above use case is still supported, or bar.cr is considered as another argument.

[asterite]

I think having run and script is very confusing. How do you explain the difference in easy words, and why run and script do different things? Can script be implicit? Why is it needed if crystal file.cr already works?

[RX14]

The current compiler syntax is a mess.

$ echo 'puts "hi"' > foo.cr
$ echo 'puts "world"' > bar.cr
$ crystal run foo.cr bar.cr   
hi
world
$ crystal foo.cr bar.cr    
hi

Adding crystal -Dfoo file.cr (assuming you want more than -D) would take a lot more work than this PR takes, which is why I didn't add it to this PR.

crystal run foo.cr arg already passes arg as a parameter - it passes anything which doesn't start with a - as an argument, and everything which starts with a - it tries to parse. This is inconsistent but I left the behaviour as-is for compatibility. I changed only crystal file.cr as it was lesser used, and the only option which is usable for shebangs because of what I mentioned above.

I agree with all of you that this area of the compiler has serious flaws and needs review, but it needs a lot more work than can comfortably fit in this PR, and so I'd like to merge an improvement...

[RX14]

Here is the current logic the compiler uses to determine whether to half-emulate crystal run:

crystal/src/compiler/crystal/command.cr

Line 116 in c2113aa

run_command(single_file: true)

I'm really not sure how to add options to this, since crystal -Dfoo build foo.cr turning into crystal run -Dfoo build -- foo.cr is not what we want at all.

[asterite]

crystal run foo.cr arg already passes arg as a parameter - it passes anything which doesn't start with a - as an argument, and everything which starts with a - it tries to parse. This is inconsistent but I left the behaviour as-is for compatibility

I don't think we should leave this inconsistency right before 1.0. Is it easy to fix? Everything after the filename should be an argument to the program, and everything before the filename should be an argument to the compiler (according to what seems to be desired).

[bcardiff]

No @asterite , because crystal run foo.cr --release is desired.
Also, for mixing compiler and program args you use crystal run foo.cr --release -- args and play nice with the possibility of multiple files.

Ok @RX14, we can leave compiler flags for later in script if it is an implementation detail. It will be another feature to the command and can be added later.

I think that adding crystal script will serve for simplicity to express what is the default command.

[asterite]

Just as a summary (@waj made one, but I think this is more complete):

• if you run crystal foo.cr bar.cr --release -- a then:
  • foo.cr is compiled
  • ["bar.cr", "--release", "--", "a"] is passed to the program (not to the compiler)
  • in this form there's no way to specify compiler arguments
• if you run crystal run --no-debug foo.cr bar.cr --release -- a then:
  • foo.cr and bar.cr are both compiled (leading to a single binary)
  • ["--no-debug", "--release"] is passed to the compiler
  • ["a"] is passed to the program

[RX14]

• in this form there's no way to specify compiler arguments

Note: adding this isn't a breaking change after 1.0, luckilly

[asterite]

Note: adding this isn't a breaking change after 1.0, luckilly

Though it's not clear how it would be done (or even if it's needed at all).