Using quotation marks for arguments in repl, that allows for using sp…

[unorsk]

Overview

I stumbled upon a error where I was trying to load a file (in ucm) that contained spaces in it's name. That is because currently the function words is used to split input into an array of command and arguments. So this doesn't account for any ways for using quotation or something.

I somehow expected all these to be valid ucm input (which will be valid after this PR):

.> load "scra tch.u"
.> load scratch.u
.> compile main "scra tch"
.> run someSymbol arg1 arg2 "arg 3"

And I also fixed a small typo where the symbol argument to run.file was missing (just in the help text). A very minor thing, but I just got confused when stumbled upon it.

Before:

After:

Implementation notes

I found a useful function words' that is used in ghci for a similar problem (words' is a bit smarter when it comes to quotation marks)

ghci> words' "argument1 \"argument number 2\" three"
["argument1","argument number 2","three"]
ghci> words' "argument1 \"two"
["argument1","\"two"]

Interesting/controversial decisions

The words' function was taken from ghci. I guess it might lead to some licensing implications, theoretically. But I just couldn't find a better option. So tbh I don't know if it is ok or not.

Test coverage

I haven't included any tests.
But I did some manual testing :)
The change affects all repl commands, so it would be nice if someone else could try running this.

Loose ends

None :)

[runarorama]

Regarding copying source from ghci, I think the thing to do is just point to https://hackage.haskell.org/package/ghci from CREDITS.md and link to the BSD3 license.

[unorsk]

Regarding copying source from ghci, I think the thing to do is just point to https://hackage.haskell.org/package/ghci from CREDITS.md and link to the BSD3 license.

Cool! I can add that!
Any comments on the solution itself? :)

[aryairani]

@mitchellwrosen Could you still take a look at this one 🙏

[mitchellwrosen]

Sorry for the delay; it's kind of hard for me to think through all the implications, but I'm leaning towards "this change makes sense".

It does seem to me that only a couple commands might want to escape a space in this way, namely those that take a filename, and that a more general escaping syntax is something we want/need anyway, for Unison identifiers.

Just thinking out loud, what if I wanted a command that writes a Unison term by name to a file, with some backslash syntax for weird characters? It might look like:

> write-term-to-file the\-\"weird\"\-unison\-term "the filename"

So, perhaps something more sophisticated is needed to support that kind of thing, if we ever get there, or want to get there. But doing a GHCi thing on all arguments seems ok in the interim.

Unfortunately, there are conflicts 😅 @unorsk would you mind taking a look?

[mitchellwrosen]

And just to briefly argue the other side, doing nothing here seems reasonable to me, too. I don't think supporting filenames with spaces in them is very high on the priority list, unless for some reason we encounter a use case where the user has no control over the filename in question.

[ceedubs]

It does seem to me that only a couple commands might want to escape a space in this way, namely those that take a filename

@mitchellwrosen run main "my first arg" "my second arg" is a thing that people might want to do.

#2805 is an example of an issue that this could help with. Though that could potentially be solved by making only the first argument of run treated as a potential term reference.

[unorsk]

Merged conflicts. Just in case :)

PS Sorry, I somehow managed to miss notifications for this PR 😅

[aryairani]

I think it was an accident that this slipped through the cracks and isn't merged already; and now there are conflicts again. :-\

@mitchellwrosen Could you help get this PR over the finish line in some form or other? For now the goal is to support normal-ish program args to run. If we are trying to run a definition that has a `"` in the name, it's okay if it fails in this first iteration.

[mitchellwrosen]

Circling back to this one, I think perhaps we should go in a different direction: rather than make all commands use the same tokenizer (previously words, in this PR some variant of words that knows about double quotes), I think we should let commands implement their own tokenizer.

That way, run <definition name> <arg> <arg> <arg> can just work without changing the behavior of other commands.

[mitchellwrosen]

@unorsk I plan on picking this up from here, thank you for the contribution; I will rebase your commits atop main and go from there, so you will be credited in the repo history.