Implemented help commands and added notice thereof to help output

[rben01]

No description provided.

[sharkdp]

I'm not 100% satisfied with the phrasing here. Numbat (the language) doesn't really support any commands. You can't write quit in a Numbat program, for example. It's really the different REPL/CLI interfaces that support certain commands. And that leads me to the other problem here: with the current implementation, this message will also be displayed in the web version. But help commands doesn't work there, and if it did, it would have to show a different list of commands.

I really like this new feature and the help commands output looks great! But I think we need to work on the web version first. As I said elsewhere, it would be great if we could share command-parsing code between different "frontends". We would then need a way to customize (1) which commands are available and (2) how some of them are implemented in a particular frontend, e.g. using a trait or using simple callbacks.

In some sense, the whole command-parsing/handling thing probably doesn't even belong in the numbat crate. It's not part of the compiler or the language implementation. Even now, there are some frontends (IRC bot, mobile app, Jupyter kernel) that don't make use of commands. So they could be moved to a separate crate (numbat-commands or similar). But I guess for simplicity, we can also leave the code in the numbat crate for now. We also have things like unicode_input.rs that are only used in specific frontends.

[rben01]

Thanks, you're right. I'm definitely too CLI-centric 😄. I'll need to think about how to share almost-the-same code between frontends. A numbat-commands crate probably does sense, with features to enable/disable various commands. Then in the commands code, just slap a #[cfg(feature)] on every branch of every match statement.

[sharkdp]

I had something else in mind. Let's maybe discuss different options first. I'll try to write down what I thought about tomorrow.

[rben01]

I had something else in mind. Let's maybe discuss different options first. I'll try to write down what I thought about tomorrow.

Curious to hear what your idea was.

[sharkdp]

I'm sorry, I was busy over the last few weeks. I was thinking that we could have a commands module in Numbat with a Commands struct that frontends could (1) use to configure which commands they want to support (2) use to specify how they implement some of these commands and (3) use as a "preprocessor" for the actual command parsing and command-vs-Numbat-code decision.

Things like help and list would maybe be enabled by default and would need no configuration. Things like copy could be "opt in", if the frontend supplies an implementation.

One way to do this would be to add traits for some of these commands. In order to implement copy, a frontend would basically have to implement a copy_value function on a CopyCommand trait. That function would receive the value (maybe with some meta information like the type), and return either () or maybe a Result<(), SomeCustomError>.

I'm thinking something like:

struct CliCopyCommand;

impl commands::CopyCommand for CliCopyCommand {
  // ...
}

struct CliClearCommand;
// …

struct CliSaveCommand;
// …

let commands = CommandConfiguration::new()
    .enable_copy(CliCopyCommand {})  // or maybe .enable_copy::<CliCopyCommand>()
    .enable_clear(…)
    .enable_save(…)
    .build();

// In the REPL loop:

if !commands.run_command(input) {
  // not a command => interpret it as Numbat code
  context.interpret(input)
}

Now that I write it down, it looks quite verbose. Maybe we can replace the traits by just simple callables/closures.

And everything else is also open to being redesigned.

[rben01]

If we're ok storing everything in the context, then that will work. It still feels a bit odd to me to defer all of this to runtime when it could be gated behind compile-time features — what's the rationale for that? Is it desirable to dynamically choose the commands they want at initialization? I am under the impression that each frontend will fill in its CommandConfiguration with compile-time constants, which is equivalent to feature flags but without the compiler checking your work for you.

[sharkdp]

If we're ok storing everything in the context, then that will work.

What is "everything" except for the last value?

It still feels a bit odd to me to defer all of this to runtime when it could be gated behind compile-time features

what is "all of this"? It's the code for the CommandsBuilder struct. All the heavy stuff (like a whole clipboard library with all its dependencies) is going to come from the frontend. The backend (numbat crate) will only provide some hooks to set up this functionality. I fail to see how this would be a huge cost.

Compile-time features are a great thing, but I try to avoid them wherever possible. It makes testing much more complicated, because you have an exponentially growing set of variants of your program.

but without the compiler checking your work for you.

What part wouldn't be safe in the dynamic version of this?

[rben01]

What is "everything" except for the last value?

Session history as well, and who knows what else in the future.

what is "all of this"? It's the code for the CommandsBuilder struct. All the heavy stuff (like a whole clipboard library with all its dependencies) is going to come from the frontend. The backend (numbat crate) will only provide some hooks to set up this functionality. I fail to see how this would be a huge cost.

I'm referring to the various fields and logic necessary for each bit of command functionality. I agree that it's not “heavy”, but nevertheless there is logic that the compiler won't be able to check.

What part wouldn't be safe in the dynamic version of this?

Nothing will be unsafe, it's only needless work that might be done.

On the whole the difference is minor, and I understand wanting to do this at runtime instead of compile time for ease of testing. I guess I think that while the mechanics of testing are easier (you don't need a large test matrix in GitHub actions or whatever), I prefer to have the compiler check everything, including which fields even exist on Context. But I don't feel strongly enough about this to block further progress on this.

[sharkdp]

I might be wrong here, but I still lean towards the "dynamic" solution. I would probably store the session history in the Commands struct.

Or we could leave storage (in memory) of the history to the client. And have something like commands.with_history_storage(…) that would accept a HistoryStorage trait argument with a suitable interface. And then Commands would push single entries into that storage and look up things when needed. But there are some questions as to how that would interact with the save command etc.

I think at some point we just need to try it. I'm definitely not asking you to do this, especially if you would rather go for a different implementation. But that's what I would probably do next. Just try out an implementation and see if it works. Or build a small prototype independent from Numbat.

[rben01]

I have made an initial implementation in #663