Adds new -e and --eval options which will immediately execute a script #1129

glennblock · 2015-12-28T01:08:08Z

This fixes #1085

Update: -e was chosen to align with the existing *nix precedent. Mono's REPL uses this and node as well. Thanks to @migueldeicaza for the suggestion!

It adds a new switch for immediately executing loose code. Also adds a new convenience overload to the ScriptCsExe.Run() method.

example:

scriptcs -e 'Console.WriteLine("""Hello from scriptcs""");'

Note: Depending on the shell you use you may have to jump through different hoops for passing spaces / quoted strings. The example is using Powershell. To do this from the cmd prompt you would use:

scriptcs -e 'Console.WriteLine(\"\"\"Hello from scriptcs\"\"\");'

filipw · 2016-02-28T22:30:14Z

src/ScriptCs/Command/ExecuteLooseScriptCommand.cs

+
+namespace ScriptCs.Command
+{
+    internal class ExecuteLooseScriptCommand : IExecuteLooseScriptCommand


this is almost a copy of ExecuteScriptCommand, the only difference appears to be _scriptExecutor.Execute vs _scriptExecutor.ExecuteScript.
It would be better to add a common abstract class and share most of the logic

Ok sounds good.

filipw · 2016-02-28T22:33:11Z

@glennblock this is great, very welcome option. What I do in a scriptcs extension for VS Code, to allow script snippet evaluation is I saved it to a temp file (lol https://github.com/filipw/vscode-scriptcs-runner/blob/master/src/extension.ts#L81) and invoke that haha so it's very welcome

Just left a comment to avoid code copying/duplication

glennblock · 2016-02-28T22:52:15Z

Great suggestion. Glad you find it useful.

filipw · 2016-02-29T00:16:44Z

Also, according to TC this PR has 28 failing tests, so can you please double check everything passes OK

adamralph · 2016-02-29T00:20:01Z

@filipw it's because of this #1127

glennblock · 2016-02-29T00:22:37Z

Yeah it is very inconsistent that this path error pops up
On Sun, Feb 28, 2016 at 4:20 PM Adam Ralph notifications@github.com wrote:

@filipw https://github.com/filipw it's because of this #1127
#1127

—
Reply to this email directly or view it on GitHub
#1129 (comment).

adamralph · 2016-02-29T00:43:39Z

It probably depends which build agent it runs on.

adamralph · 2016-02-29T01:57:27Z

Scratch that. The test failures are genuine. I can reproduce them locally.

They were introduced with the merge of #1134

If I build the commit on the dev branch immediately before that merge, everything is fine.

filipw · 2016-02-29T02:02:13Z

how is that possible, given that the build status reported on that PR was green

glennblock · 2016-02-29T02:06:53Z

@adamralph looking into this now

adamralph · 2016-02-29T02:09:54Z

Only the Travis build was allowed to run before merging, TC was still to kick in. Why the tests are passing in Linux and not in Windows, I'm not sure.

glennblock · 2016-02-29T02:15:47Z

@adamralph I see the errors. I'm going to fix in the original branch and send another PR.

glennblock · 2016-02-29T02:54:30Z

Working on this, I think I see the issue.

On Sun, Feb 28, 2016 at 6:09 PM Adam Ralph notifications@github.com wrote:

[image: image]
https://cloud.githubusercontent.com/assets/677704/13383990/1532a020-de91-11e5-9755-cf75c68cd708.png

Only the Travis build was allowed to run before merging, TC was still to
kick in. Why the tests are passing in Linux and not in Windows, I'm not
sure.

—
Reply to this email directly or view it on GitHub
#1129 (comment).

glennblock · 2016-02-29T04:26:43Z

Fixed. I'll send a new PR shortly.

On Sun, Feb 28, 2016 at 6:54 PM Glenn Block glenn.block@gmail.com wrote:

Working on this, I think I see the issue.

On Sun, Feb 28, 2016 at 6:09 PM Adam Ralph notifications@github.com
wrote:

[image: image]
https://cloud.githubusercontent.com/assets/677704/13383990/1532a020-de91-11e5-9755-cf75c68cd708.png

Only the Travis build was allowed to run before merging, TC was still to
kick in. Why the tests are passing in Linux and not in Windows, I'm not
sure.

—
Reply to this email directly or view it on GitHub
#1129 (comment).

glennblock · 2016-03-02T04:51:01Z

@adamralph @filipw all tests passing now based on my last DOH fix.

glennblock · 2016-03-02T06:15:10Z

@adamralph any idea why TC seems to be slow in picking up the changes?

glennblock · 2016-03-02T06:15:29Z

@filipw ready to merge!

adamralph · 2016-03-02T08:57:11Z

@adamralph any idea why TC seems to be slow in picking up the changes?

@glennblock Codebetter CI is just slow 😒

glennblock · 2016-03-04T06:50:52Z

@scriptcs/core this is waiting on someone to click merge ;-)

glennblock · 2016-03-04T17:42:33Z

@adamralph Travis is lying

adamralph · 2016-03-05T11:27:26Z

@glennblock I kicked off the build again and it passed.

adamralph · 2016-03-05T11:34:55Z

src/ScriptCs/Command/ExecuteLooseScriptCommand.cs

+
+                var assemblyPaths = Enumerable.Empty<string>();
+                var workingDirectory = _fileSystem.CurrentDirectory;
+                assemblyPaths = _assemblyResolver.GetAssemblyPaths(workingDirectory);


The declaration and assignment of assemblyPaths can be combined on this line.

You are being a little anal here

adamralph · 2016-03-05T11:40:45Z

test/ScriptCs.Tests.Acceptance/LooseScriptExecution.cs

+                    directory = ScenarioDirectory.Create(scenario);
+                    if (Environment.OSVersion.Platform == PlatformID.Win32NT)
+                    {
+                        script = @"Console.WriteLine(""""""Hello World!"""""");";


Why does Windows need 3 double quotes?

Because it does. This has been thoroughly tested and it is apparently by design.

adamralph · 2016-03-05T11:53:56Z

@glennblock I left a couple of comments regarding the implementation.

Regarding behaviour, I'd like to investigate the result of combinations of a filename, an eval script and the REPL switch:

File	Script	`-R`	Result
			REPL
X			file, exit
	X		script, exit
X	X		script, exit¹
		X	REPL
X		X	file, REPL
	X	X	script, exit²
X	X	X	script, exit³

We are saying that a script trumps a file. That's fine, but out of interest, how easy would it be to run both in the same session? If possible I guess we'd have to say which comes first, file or script?
I think this is problematic. If I pass a file and -R, the file is executed and then I get REPL. I think it's astonishing that when pass a script and -R that I don't get a REPL.
This is a combination of points 1 and 2.

glennblock · 2016-03-05T16:33:32Z

I see your point, but these seem like small issues to me. The manor use case here is to allow immediately executing a command that is passed at the command line. The other combinations are nice to have, sure they bring more consistency, but still nice to have and unlikely to be used.

Using -r however is there to allow pre-loading a bunch of stuff when you come into the REPL so it executes and then you can use the loaded objects. This feels far less valuable in this context.

glennblock force-pushed the 1085 branch 2 times, most recently from 4a42570 to 14abb8f Compare December 28, 2015 02:37

glennblock changed the title ~~Adds new -x and -exec options which will immediately execute a script~~ Adds new -e and -exec options which will immediately execute a script Dec 28, 2015

glennblock changed the title ~~Adds new -e and -exec options which will immediately execute a script~~ Adds new -e and --eval options which will immediately execute a script Feb 14, 2016

glennblock mentioned this pull request Feb 28, 2016

Renames ScriptCs.CSharp to ScriptCs.Engine.Roslyn #1134

Merged

filipw reviewed Feb 28, 2016
View reviewed changes

Adds new -e and -eval options which will immediately execute a script

5d11677

glennblock force-pushed the 1085 branch from 5decd9a to 5d11677 Compare March 3, 2016 15:33

adamralph reviewed Mar 5, 2016
View reviewed changes

glennblock merged commit e31b194 into scriptcs:dev Dec 29, 2016

glennblock deleted the 1085 branch January 3, 2017 00:12

Adds new -e and --eval options which will immediately execute a script #1129

Adds new -e and --eval options which will immediately execute a script #1129

Uh oh!

Conversation

glennblock commented Dec 28, 2015

Uh oh!

filipw Feb 28, 2016

Choose a reason for hiding this comment

Uh oh!

glennblock Feb 28, 2016

Choose a reason for hiding this comment

Uh oh!

filipw commented Feb 28, 2016

Uh oh!

glennblock commented Feb 28, 2016

Uh oh!

filipw commented Feb 29, 2016

Uh oh!

adamralph commented Feb 29, 2016

Uh oh!

glennblock commented Feb 29, 2016

Uh oh!

adamralph commented Feb 29, 2016

Uh oh!

adamralph commented Feb 29, 2016

Uh oh!

filipw commented Feb 29, 2016

Uh oh!

glennblock commented Feb 29, 2016

Uh oh!

adamralph commented Feb 29, 2016

Uh oh!

glennblock commented Feb 29, 2016

Uh oh!

glennblock commented Feb 29, 2016

Uh oh!

glennblock commented Feb 29, 2016

Uh oh!

glennblock commented Mar 2, 2016

Uh oh!

glennblock commented Mar 2, 2016

Uh oh!

glennblock commented Mar 2, 2016

Uh oh!

adamralph commented Mar 2, 2016

Uh oh!

glennblock commented Mar 4, 2016

Uh oh!

glennblock commented Mar 4, 2016

Uh oh!

adamralph commented Mar 5, 2016

Uh oh!

adamralph Mar 5, 2016

Choose a reason for hiding this comment

Uh oh!

glennblock Mar 5, 2016

Choose a reason for hiding this comment

Uh oh!

adamralph Mar 5, 2016

Choose a reason for hiding this comment

Uh oh!

glennblock Mar 5, 2016

Choose a reason for hiding this comment

Uh oh!

adamralph commented Mar 5, 2016

Uh oh!

glennblock commented Mar 5, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants