zq: Better error messages for single argument commands

[mattnibs]

Fixes #5046
Fixes #4731

[philrz]

I just tested this branch out at commit f361f11 using the repro example from #5046.

$ zq -version
Version: v1.14.0-7-gf361f112

$ zq -Z 'fork(=> file "./csv_1.csv" | yield email | collect(this), => file "./csv_2.csv" | yield this["Recommended Email"])'
zq: could not invoke zq with a single argument because:
 - the argument did not parse as a valid Zed query
 - a file could not be found with the name "fork(=> file \"./csv_..."

This seems like a great improvement to me so I'm functionally 👍 on this.

As @mattnibs and I have discussed offline, the user would surely appreciate being able to see the parse error in a situation like this. Two thoughts come to mind:

1. We could present it as part of this two-part message. If we do, I might suggest swapping the order since I expect the parse error will take up multiple lines.
2. We could implement the -query flag as mention in the docs and zq "-query" flag mentioned in docs but doesn't exist #5052 and make some reference to it in this error message (e.g., "consider -query").

[philrz]

I just re-tested the branch now at commit 2699090 once again using that example from #5046. It looks great with the parse error!

$ zq -version
Version: v1.14.0-8-g2699090d

$ zq -Z 'fork(=> file "./csv_1.csv" | yield email | collect(this), => file "./csv_2.csv" | yield this["Recommended Email"])'
zq: could not invoke zq with a single argument because:
 - a file could not be found with the name "fork(=> file \"./csv_..."
 - the argument could not be compiled as a valid Zed query due to parse error (column 58):
   fork(=> file "./csv_1.csv" | yield email | collect(this), => file "./csv_2.csv" | yield this["Recommended Email"])
                                                         === ^ ===

I then thought to see how it would look if I had some multi-line Zed.

$ zq -Z 'fork (
>   => file "./csv_1.csv" | yield email | collect(this),
>   => file "./csv_2.csv" | yield this["Recommended Email"]
> )'
zq: could not invoke zq with a single argument because:
 - a file could not be found with the name "fork (\n  => file \"./..."
 - the argument could not be compiled as a valid Zed query due to parse error (line 2, column 2):
     => file "./csv_2.csv" | yield this["Recommended Email"]
   = ^ ===

That's great since it mentions the line & column but shows just the line that had the error. Very nice! 👍

(I guess it's a partial bummer that the parser doesn't point directly at the , as being the problem, but that doesn't appear to be this PR's fault.)

[nwt]

This is pretty wordy. How about something more concise?

zq: not a file or valid query: "from ( file a ) | co..."
  error parsing Zed at column 25:
  from ( file a ) | count(
                      === ^ ===

As a bonus, that would make for a much simpler singleArgError.

func singleArgError(src string, err error) error {
	if len(src) > 20 {
		src = src[:20] + "..."
	}
	msg := fmt.Sprintf("not a file or valid query: %q", src)
	if errors.As(err, new(*parser.Error)) {
		msg += "\n" + err.Error()
	}
	msg = strings.ReplaceAll(msg, "\n", "\n   ")
	return errors.New(msg)
}

[nwt]

(To be clear, I'm OK with the existing message if you guys prefer it.)

[philrz]

Thanks @nwt. Indeed, I was in the middle of writing a reply when that last comment of yours came in, so I'll paste that below for completeness, but basically, yeah, I happen to prefer what's already there. 😄

I know I can tend toward wordiness in my own work and so I try to keep that in check, but I think this is one place where verbosity is maybe justified. For instance, in the #5046 case the user is almost certainly thinking entirely in terms of their argument being a Zed query so I'm concerned something concise like "not a file" might still be confusing. "Not a file? What? The file refs are in my program. I don't get it." This particular error is triggered rarely enough and has confused users enough in is previous form in enough that I feel pretty strongly that the benefits of super clear communication outweigh any assumed benefits from being concise.

Come to think of it, the user that last hit #5046 is responsive on Slack today, so I'll see if they're game to weigh in to the discussion, though if we already have consensus and want to get on with it and merge I'm fine with that too.

[nwt]

@mattnibs: Any preference here?

[mattnibs]

I am perhaps biased here, and while I appreciate the simplicity of the code change of your solution, I perfer the formatting the way it is in this pr. To my eyes having the errors printed out in list format makes it easier for me to understand that the error is a result of one of two conditions.

[iloveitaly]

fwiw, as someone who doesn't use zed every day, but reaches for it probably once a week when manipulating data I'd prefer the more verbose message. It's hard to understand exactly what might be wrong, especially given the unfamiliarity of the zq syntax compared to other tools.