Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jq: bump up gojq, better query parse error, handle halt error gracefully #155

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Apr 1, 2024

The error of invalid --jq query is difficult to know what's wrong with the query. As a gojq library maintainer, I was wondering whether the library should return pretty error or not, but since the query input is given by library user, returning an error with offset is enough as a library, just like json.SyntaxError. In the release v0.12.15, I decided to export gojq.ParseError, which has the offset position in the query. This PR improves the parsing error message to report that position in fancy style. It also improves handling of halt errors.

 $ gh api /user --jq '[1,2,3,,4,5]'    
unexpected token ","

 $ bin/gh api /user --jq '[1,2,3,,4,5]'
failed to parse jq expression (line 1, column 8)
    [1,2,3,,4,5]
           ^  unexpected token ","

@williammartin williammartin self-requested a review April 2, 2024 18:09
Copy link
Member

@williammartin williammartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great contribution @itchyny. Thanks a lot for maintaining gojq and helping us use it better!

Here's a comparison between current release, and this PR:

➜  ~ gh api /user --jq '. | [1,,3]'
unexpected token ","
failed to parse jq expression (line 1, column 8)
    . | [1,,3]
           ^  unexpected token ","

That's really nice!

@@ -65,6 +75,10 @@ func EvaluateFormatted(input io.Reader, output io.Writer, expr string, indent st
break
}
if err, isErr := v.(error); isErr {
var e *gojq.HaltError
if errors.As(err, &e) && e.Value() == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For anyone else wondering what this is about, from the gojq docs:

In any case, it is recommended to stop the iteration on gojq.HaltError, which is emitted by halt and halt_error functions, although these functions are rarely used. The error implements gojq.ValueError, and if the error value is nil, stop the iteration without handling the error. Technically speaking, we can fix the iterator to terminate on the halting error, but it does not terminate at the moment. The halt function in jq not only stops the iteration, but also terminates the command execution, even if there are still input values. So, gojq leaves it up to the library user how to handle the halting error.

This is handing the case when the jq expression contains halt (exit with no error), which previously we wouldn't have respected.

@williammartin williammartin merged commit a69ba78 into cli:trunk Apr 3, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants