testscript: Accept !cmd (without space) or provide better error

[awagner-iq]

A colleague is trying out testscript and used !cmd to call a custom command (note the lack of space). They got the error message "!cmd not defined" and where confused why the negation was not recognized. I'd like to improve that.

One option would be to accept the !cmd, parsing it as equivalent to ! cmd. This is easy to implement, but probably not backwards compatible.

Another option is to provide a better error message if a command is not found. e.g. if !cmd is looked up and not found, but cmd is, we could detect that and report "did you mean ! cmd?". Or - harder to implement, but more generally useful - we could search the defined commands by edit-distance to the missed command and if one of them is a close enough match, suggest that. x/tools has an internal package that might help.

I can send a PR, but wanted to discuss what option you prefer.

[ldemailly]

I hit this as well and I think treating it as ! cmd is the right solution. Why would someone name a binary starting with a ! (which is special in shell as well)

[myitcv]

Another option is to provide a better error message if a command is not found

I would tend towards this approach but defer to @rogpeppe and @mvdan

[ldemailly]

did same typo with !stderr also so I really think the not should work without needing a space

[mvdan]

Giving a better error message seems fine to me. I wouldn't want to change the syntax to allow !foo as an alias for ! foo, because that would take our syntax further apart from upstream for little benefit. I think the consistency and readability of the spacing is also nice.

Personally I'd say that erroring on any command with a ! prefix would be fine, because such a command sounds prone to confusion and should be discouraged, and I doubt anyone uses them currently. But if we're really concerned with backwards compatibility, only improving the error when we run into "no such command" also seems fine.

Happy to review a PR :)

[Merovius]

I think I'll try my hand at a general spelling-correction to improve the error message (which should cover this case as well). If I can't manage that, I'll send a PR to detect !cmd vs. ! cmd specifically.

[ldemailly]

curious btw

that would take our syntax further apart from upstream

what upstream are you referring to ?

also noticed by the way that lines like

[!exec:false] skip

aren't

[ ! exec:false ] skip

[mvdan]

what upstream are you referring to ?

Currently https://github.com/golang/go/blob/master/src/cmd/go/script_test.go. testscript was started as a copy of that code while being refactored as a library. See all the testscripts upstream has, where the syntax is still very similar.

[thepudds]

Hi @Merovius, just wanted to say a quick thanks for doing this!

I hit this just yesterday, and the time between making the mistake to then smacking my head once I realized the mistake was longer than I would care to admit 😅