Skip to content

Replcommands#612

Closed
filipw wants to merge 14 commits into
scriptcs:devfrom
filipw:replcommands
Closed

Replcommands#612
filipw wants to merge 14 commits into
scriptcs:devfrom
filipw:replcommands

Conversation

@filipw
Copy link
Copy Markdown
Member

@filipw filipw commented Apr 3, 2014

Overview

This PR introduces the infrastructure for REPL commands. Fixes #224 and #237. The design is as discussed in #583.

The command is defined as follows:

public interface IReplCommand
 {
     string CommandName { get; }
     object Execute(IScriptExecutor repl, object[] args);
 }

And can either execute arbitrary code, or interact with the REPL (has access to IScriptExecutor). If it returns the value, it's printed to the REPL too. It should now enable easy extensiblity i.e. do something like requested in #402 or #427

The interesting thing is that you can also pass context variables to the REPL. For example:

   > var x = 1;
   > :myCommand hello x

Will pass hello string and 1 to command. This is done by evaluating the argument with the ScriptEngine- if it returns a value, use it, otherwise assume it's just a string.

Commands included

Converted the existing REPL commands to this approach:

  • CwdCommand (show working directory)
  • CdCommand (change workign directory)
  • ClearCommand (cls)
  • ResetCommand (reset executor)

Also added InstallCommand - this fixes #609.
You can now do:

  • :install myPackage
  • :install myPackage 0.3
  • :install myPackagae 0.3 -pre

Extra points

  • currently the infra loads all commands it can find in the AppDomain. The idea was that as long as you have a DLL with IReplCommand, it should be discovered. This is a bit too invasive, so we may want to change it for a better approach. At the moment it's not integrated with Modules, but it should be trivial to do.
  • right now all of the commands are loaded with : - I think it's actually better to have a uniform way rather than have a couple of commands starting with # because Roslyn does that, and have the rest start with :. But of course it would be possible to have "system" commands that start with #.
  • The code to parse command is now embedded into the Repl class. There are better ways to do it but I didn't want to do too much in one PR. See issue Make REPL injectable and overrideable like other services #434 - I think this should be tackled next.
  • I have plenty of other examples if there is some interest in extending it further - some mentioned in Using scriptcs as a liveable shell? #583 already - i.e. HttpCommand or Command to build other commands inline :-)

Let me know what you think!

Comment thread src/ScriptCs.Hosting/RuntimeServices.cs Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should filter out abstract classes? And also, shouldn't this be combined with discovery of other types (through MEF?)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes good point

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And if we stick with this, maybe GetExportedTypes instead of GetTypes, we want to respect types' privacy 😉

@glennblock
Copy link
Copy Markdown
Contributor

@filipw can you bring this up to date?

@filipw
Copy link
Copy Markdown
Member Author

filipw commented May 26, 2014

Fixed some issues and updated to latest, so it's mergeable again.
It works on mono too, but will display some extra unintentional warnings due to #664.

@adamralph adamralph mentioned this pull request May 29, 2014
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the reason for this change?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was not properly checked all along, when not possible to parse a version from string, version is set to new empty Version, which means minor 0, major 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In that case should we not check packageRef.Version != new Version()?

…ommands

Conflicts:
	src/ScriptCs.Core/Repl.cs
	src/ScriptCs/Command/ExecuteReplCommand.cs
@khellang
Copy link
Copy Markdown
Member

#726 just landed. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nuget package installation from the REPL REPL - Basic Commands

4 participants