Skip to content
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

Start discussing design around dynamic commands and command providers #195

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,6 @@ src/TypinExamples/TypinExamples/wwwroot/assets/libraries/*/*
!src/TypinExamples/TypinExamples/wwwroot/assets/libraries/chota/about.txt
!src/TypinExamples/TypinExamples/wwwroot/assets/libraries/font-awesome/about.txt
!src/TypinExamples/TypinExamples/wwwroot/assets/libraries/xterm/about.txt
!src/TypinExamples/TypinExamples/wwwroot/assets/libraries/monaco-editor/about.txt
!src/TypinExamples/TypinExamples/wwwroot/assets/libraries/monaco-editor/about.txt

/.ionide
174 changes: 174 additions & 0 deletions src/Typin/Typin/Experimental/Design.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace Typin.Experimental
{
// Command Specification

/// <summary>
/// This interface represents a type of command where its <see cref="Descriptor"/> describes
/// the class of command (with summary, options, arguments, examples, etc.).
///
/// This command type could represent just a generic description for multiple other sub-commands,
/// and not be able to run itself. However it could add some description and examples for the entire
/// collection of sub-commands, and add some common arguments and options accesible for all sub-commands
///
/// A <see cref="ICommandProvider"/> returns a collection if this type to indicate available
/// commands to the Typin runtime.
/// </summary>
public interface ICommandType
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// Returns a Command Type description that includes human readable information and
/// arguments and options specifications
/// </summary>
public CommandDescriptor Descriptor { get; }
}

/// <summary>
/// This interface represents a type of command that can perform a specific task. Hence it inherits
/// it's description from <see cref="ICommandType"/> and adds through <see cref="CreateRunner"/>
/// a way to obtain a new instance of a command runner.
/// </summary>
public interface IExecutableCommandType : ICommandType
Copy link
Owner

Choose a reason for hiding this comment

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

So it looks like ICommandType is CommandSchema and IExecutableCommandType is ICommand. Why we have inheritance here? These are two separate things, and I don't like a concept of mixing them. If someone wants "Typin reflection" aka schemas, there is a service for this.

Copy link
Author

Choose a reason for hiding this comment

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

I think with my previous comment above this was made clear. We could even skip the distinction and make a ICommandSchema return null when CreateRunner is called to indicate that there is nothing to run, or have that information explicitly in the CommandDescriptor with a property bool IsAbstract. This is a detail for later.

{
/// <summary>
/// Creates and returns a new instance of <see cref="ICommandRunner"/> capable of executing
/// the task this type represents.
/// </summary>
/// <returns></returns>
ICommandRunner CreateRunner();
}

/// <summary>
/// Describes a <see cref="ICommandType"/> with human readable information and command input specifications
/// </summary>
public class CommandDescriptor
{
/// <summary>
/// Default name to invoke the command. It can be a space separated collection of tokens to create
/// a command hierarchy, like <code>pods</code> and <code>pods list all</code>
/// </summary>
public string Name { get; init; }

/// <summary>
/// A short summary able to be printed in a single console line, when the command is presented
/// in a list of other sibling commands
/// </summary>
public string Summary { get; init; }

/// <summary>
/// A description, possibly more extense than the <see cref="Summary"/> decribing what the command does
/// </summary>
public string Description { get; init; }

/// <summary>
/// List of argument descriptors specifying the arguments of current command
/// </summary>
public IReadOnlyCollection<CommandArgumentDescriptor> Arguments { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// List of option descriptors specifying the options of current command
/// </summary>
public IReadOnlyCollection<CommandOptionDescriptor> Options { get; init; }

/// <summary>
/// List of examples to show to the user for current command
/// </summary>
public IReadOnlyCollection<CommandExampleDescriptor> Examples { get; init; }
adambajguz marked this conversation as resolved.
Show resolved Hide resolved
}

public class CommandArgumentDescriptor
{
public string Name { get; init; }
public string Summary { get; init; }
public string Description { get; init; }
public int Position { get; init; }
public bool Multiple { get; init; }
}

public enum OptionKind
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
{
Flag,
Value,
}

public class CommandOptionDescriptor
{
public string Name { get; init; }
public IReadOnlyCollection<string> ShortAliases { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// For flags, you can specify a negative name, like no-verbose, no-build, etc., so the user can
/// explicitly indicate that he wants to give a false value for the flag, disregarding it's default value
/// </summary>
public string NegativeName { get; init; }
adambajguz marked this conversation as resolved.
Show resolved Hide resolved
public string Summary { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
public string Description { get; init; }
public bool Multiple { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
public OptionKind OptionKind { get; init; }
public string DefaultValue { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
}

public class CommandExampleDescriptor
{
public string Description { get; init; }
}

// Pure Base Runtime

public interface ICommandRunner
{
ValueTask ExecuteAsync(CommandRunnerContext context, CancellationToken cancellationToken);
}

public class CommandRunnerContext
{
public ICliContext CliContext { get; init; }
public CommandDescriptor CommandDescriptor { get; init; }
public string CommandLine { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
public IReadOnlyCollection<ParsedArgument> ParsedArguments { get; init; }
public IReadOnlyCollection<ParsedOption> ParsedOptions { get; init; }
}

public class ParsedArgument
{
public CommandArgumentDescriptor Descriptor { get; init; }
Copy link
Owner

Choose a reason for hiding this comment

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

I don't know how will you know CommandArgumentDescriptor aka CommandParameterSchema/CommandOptionSchema while parsing.

Copy link
Author

Choose a reason for hiding this comment

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

That's why I created the class CommandRunnerContext which is the context in which current command is running, and that context is prepared after converting the IEnumerable<string>, using the CommandDescriptor that corresponds to the typed command, into a parsed command, which know the CommandDescriptor and all the CommandParameterDescriptor and CommandOptionDescriptor used to parse the command line.

So the ICommandRunner, instead of receiving a ICliContext, would receive a more specialized context with more information after the parsing have happened.

Copy link
Owner

@adambajguz adambajguz Apr 11, 2021

Choose a reason for hiding this comment

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

That's why I created the class CommandRunnerContext (...) - ICliContext/CliContext does exactly the same :) However, maybe ICliContext/CliContext design is not 100% ideal. But I'm planning to change it in Typin 4.0 due to GenericHost usage, and different middlewarre configuration (more like what we have in ASP.NET Core)

Copy link
Owner

Choose a reason for hiding this comment

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

Well, maybe not exactly. We have just a raw or partially parsed input - CliContext lack a relation between input and ICommand arguments (some intermediate output from binder).

Copy link
Author

Choose a reason for hiding this comment

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

I tried to follow the basics of ASP.NET filters, where you have access to HttpContext, but also to a specific context to the filter at hand, where you have other processed data. like thrown exception, ActionDescriptor, etc.

So you have HttpContext (like ICliContext) and ExceptionContext, ActionExecutingContext and others, representing current command execution (with parsed arguments, and command metadata).

Maybe we can design it differently, but I just laid out a first approach.

public int Position { get; init; }
public string Value { get; init; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved
}

public class ParsedOption
Copy link
Owner

Choose a reason for hiding this comment

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

Well, I know you didn't want to reuse any of your types to be as succinct as possible, but this makes everything so much harder to understand. :<

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I can hope for a better understanding after a couple of iterations. If you have any idea how to converge to less friction, lets talk about it.

Copy link
Author

Choose a reason for hiding this comment

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

This is why I think that this changes are more aligned with a next version of the library, because I'm proposing a lot of changes. We can incorporate some of these to current 3.x version, and evaluate on that.

Copy link
Owner

Choose a reason for hiding this comment

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

You have amazingly thought-provoking and interesting ideas of new mechanisms. I'm thinking of writing a new concept/draft of dynamic commands myself to show my point of view, so that we can later combine what is best in our two visions. I think this will also help with disussion.

Copy link
Author

Choose a reason for hiding this comment

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

That would be great! I prefer playing with the design until it clicks, and then start building.

Also I have shown a lot of mini features but lets focus on the feature at hand: Dynamic Commands

{
public CommandOptionDescriptor Descriptor { get; init; }
public string Name { get; init; }
public string Value { get; init; }
}

// Command Providers

public interface ICommandProvider
{
IReadOnlyCollection<ICommandType> Commands { get; }
iskandersierra marked this conversation as resolved.
Show resolved Hide resolved

event EventHandler<CommandsChangedEventArgs> CommandsChanged;
}

public class CommandsChangedEventArgs : EventArgs
{
public static readonly CommandsChangedEventArgs Empty = new CommandsChangedEventArgs();
}

// Middleware

public delegate ValueTask CommandMiddlewareFunc(CommandRunnerContext context);

public interface ICommandMiddleware
{
ValueTask HandleAsync(CommandRunnerContext context, CommandMiddlewareFunc next, CancellationToken cancellationToken);
}
}