GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Adds new -e and --eval options which will immediately execute a script by glennblock · Pull Request #1129 · scriptcs/scriptcs

Conversation

@glennblock

@glennblock glennblock commented Dec 28, 2015

Copy link
Copy Markdown
Contributor

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\"\"\");'

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@glennblock
glennblock force-pushed the 1085 branch 2 times, most recently from 4a42570 to 14abb8f Compare December 28, 2015 02:37
@glennblock 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 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 glennblock mentioned this pull request Feb 28, 2016
Renames ScriptCs.CSharp to ScriptCs.Engine.Roslyn #1134
Merged
filipw
filipw reviewed Feb 28, 2016
View reviewed changes
Comment thread src/ScriptCs/Command/ExecuteLooseScriptCommand.cs Outdated

namespace ScriptCs.Command
{
internal class ExecuteLooseScriptCommand : IExecuteLooseScriptCommand

@filipw filipw Feb 28, 2016

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@glennblock glennblock Feb 28, 2016

Copy link
Copy Markdown
Contributor Author

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.

Ok sounds good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@filipw

filipw commented Feb 28, 2016

Copy link
Copy Markdown
Member

@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Feb 28, 2016

Copy link
Copy Markdown
Contributor Author

Great suggestion. Glad you find it useful.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@filipw

filipw commented Feb 29, 2016

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Feb 29, 2016

Copy link
Copy Markdown
Contributor

@filipw it's because of this #1127
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Feb 29, 2016

Copy link
Copy Markdown
Contributor Author

Yeah it is very inconsistent that this path error pops up
On Sun, Feb 28, 2016 at 4:20 PM Adam Ralph [email protected] 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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Feb 29, 2016

Copy link
Copy Markdown
Contributor

It probably depends which build agent it runs on.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Feb 29, 2016

Copy link
Copy Markdown
Contributor

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@filipw

filipw commented Feb 29, 2016

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Feb 29, 2016

Copy link
Copy Markdown
Contributor Author

@adamralph looking into this now
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Feb 29, 2016

Copy link
Copy Markdown
Contributor

image

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Feb 29, 2016

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Feb 29, 2016

Copy link
Copy Markdown
Contributor Author

Working on this, I think I see the issue.

On Sun, Feb 28, 2016 at 6:09 PM Adam Ralph [email protected] 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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Feb 29, 2016

Copy link
Copy Markdown
Contributor Author

Fixed. I'll send a new PR shortly.

On Sun, Feb 28, 2016 at 6:54 PM Glenn Block [email protected] wrote:

Working on this, I think I see the issue.

On Sun, Feb 28, 2016 at 6:09 PM Adam Ralph [email protected]
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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Mar 2, 2016

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Mar 2, 2016

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Mar 2, 2016

Copy link
Copy Markdown
Contributor Author

@filipw ready to merge!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Mar 2, 2016

Copy link
Copy Markdown
Contributor

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

@glennblock Codebetter CI is just slow 😒
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock
Adds new -e and -eval options which will immediately execute a script
5d11677
@glennblock
glennblock force-pushed the 1085 branch from 5decd9a to 5d11677 Compare March 3, 2016 15:33
@glennblock

glennblock commented Mar 4, 2016

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Mar 4, 2016

Copy link
Copy Markdown
Contributor Author

@adamralph Travis is lying
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Mar 5, 2016

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
adamralph
adamralph reviewed Mar 5, 2016
View reviewed changes
Comment thread src/ScriptCs/Command/ExecuteLooseScriptCommand.cs

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

@adamralph adamralph Mar 5, 2016

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@glennblock glennblock Mar 5, 2016

Copy link
Copy Markdown
Contributor Author

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.

You are being a little anal here

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
adamralph
adamralph reviewed Mar 5, 2016
View reviewed changes
Comment thread test/ScriptCs.Tests.Acceptance/LooseScriptExecution.cs
directory = ScenarioDirectory.Create(scenario);
if (Environment.OSVersion.Platform == PlatformID.Win32NT)
{
script = @"Console.WriteLine(""""""Hello World!"""""");";

@adamralph adamralph Mar 5, 2016

Copy link
Copy Markdown
Contributor

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.

Why does Windows need 3 double quotes?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@glennblock glennblock Mar 5, 2016

Copy link
Copy Markdown
Contributor Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@adamralph

adamralph commented Mar 5, 2016

Copy link
Copy Markdown
Contributor

@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, exit1
X REPL
X X file, REPL
X X script, exit2
X X X script, exit3
  1. 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?
  2. 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.
  3. This is a combination of points 1 and 2.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock

glennblock commented Mar 5, 2016

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glennblock
glennblock merged commit e31b194 into scriptcs:dev Dec 29, 2016
@glennblock
glennblock deleted the 1085 branch January 3, 2017 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow passing code to scriptcs at the command line

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@glennblock @filipw @adamralph
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.