Add clearer error text a few places #117
Conversation
It's nice to be clear here because it may be your first interaction with genqlient! Test plan: make check Reviewers: steve,marksandstrom
| @@ -117,8 +117,8 @@ func initConfig(filename string) error { | |||
| } | |||
| w, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) | |||
| if err != nil { | |||
| return err | |||
| return errorf(nil, "unable to write default genqlient.yaml: %v", err) | |||
There was a problem hiding this comment.
| return errorf(nil, "unable to write default genqlient.yaml: %v", err) | |
| return errorf(nil, "unable to write default genqlient.yaml: %+v", err) |
Since our errorf does its own wrapping, we don't need to use w, but the fmt.Sprintf in errorf might benefit from +v
There was a problem hiding this comment.
What would + do here? It seems to me it would only help for ints and %+q, but I may not have read the documentation carefully enough.
There was a problem hiding this comment.
(Do you mean %#v? I don't think we want the quotes here, although below they might be helpful.)
There was a problem hiding this comment.
If the value is a struct, the %+v variant will include the struct’s field names.
The %#v variant prints a Go syntax representation of the value, i.e. the source code snippet that would produce that value.
There was a problem hiding this comment.
Ah, thanks! We actually do err.Error() before passing it into Sprintf, so %+v isn't necessary. (And in general I'd like to aim for more human-readable error messages than even %+v would give.)
| } | ||
| _, err = io.Copy(w, r) | ||
| return err | ||
| return errorf(nil, "unable to write default genqlient.yaml: %v", err) |
There was a problem hiding this comment.
| return errorf(nil, "unable to write default genqlient.yaml: %v", err) | |
| return errorf(nil, "unable to write default genqlient.yaml: %+v", err) |
Since our errorf does its own wrapping, we don't need to use w, but the fmt.Sprintf in errorf might benefit from +v
| @@ -316,7 +316,8 @@ func Generate(config *Config) (map[string][]byte, error) { | |||
| // way. (As-is, we generate a broken file, with just (unused) imports.) | |||
| if len(document.Operations) == 0 { | |||
| // Hard to have a position when there are no operations :( | |||
| return nil, errorf(nil, "no queries found, looked in: %v", | |||
| return nil, errorf(nil, | |||
| "no queries found, looked in: %v (configure this in genqlient.yaml)", | |||
There was a problem hiding this comment.
Operations are slices of strings, so changing to %+v is useless at present, but if we ever pivot to making them structs, we adding the harmless + would make it Just Work™️
There was a problem hiding this comment.
I'm not sure if I see why we would make this a struct(s)? Or rather, if we did, we would want to change the error to still just print the filenames. (The snapshot-test will help us remember!)
Summary:
It's nice to be clear here because it may be your first interaction with
genqlient!
Test plan:
make check