unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.78k stars 270 forks source link

Unison crashes on attempted tab-complete #1445

Open atacratic opened 4 years ago

atacratic commented 4 years ago

I excitedly pull a juicy-looking PR:

.basememo> pull-request.load https://github.com/unisonweb/base https://github.com/anovstrup/unisonweb-base:.prs._memo

I type the following and hit tab

.basememo> todo merged.patch m

ucm crashes with no output :-(

image

Other tab-completes seem to be working for me OK.

This is master@ae95dc.

pete-ts commented 4 years ago

I think this is relevant to https://github.com/unisonweb/unison/issues/1324, due to both the arguments of todo being declared Optional.

I can raise a PR to make the first argument Required.

pete-ts commented 4 years ago

On second look, it would look like pull might suffer from the same?

https://github.com/unisonweb/unison/blob/31e19f32ebfca1d6d30194f211e91d63dae2d434/parser-typechecker/src/Unison/CommandLine/InputPatterns.hs#L685-L689

pete-ts commented 4 years ago

https://github.com/unisonweb/unison/blob/dc26ff0e07a6342e51f79e407a27a4cad1ec5e8b/parser-typechecker/src/Unison/CommandLine/InputPattern.hs#L48-L68

The issue is triggered by the "error" in the last section of the handler. todo auto => go (0, (, t) : ) = Just t todo start_typing_auto => go (0, (, t) : ) = Just t todo x start_typing => Optional parameters only work at position 0, under this countdown scheme

Changing this to Required + Optional as @anovstrup observed is probably not the right thing to do. (Although due to the nature of the countdown scheme appears to work when tested... :thinking: )

@aryairani any suggestions for how to proceed with this one? Ideally would like to add a unit test or two as well, but unclear if there is value in that.