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

blank space added #131

Merged
merged 1 commit into from
Nov 4, 2021
Merged

blank space added #131

merged 1 commit into from
Nov 4, 2021

Conversation

goessner
Copy link
Collaborator

@goessner goessner commented Nov 3, 2021

... and some minor edits ...

basic-expr = exist-expr / paren-expr / (neg-op paren-expr) / relation-expr
basic-expr = exist-expr /
[neg-op] paren-expr /
relation-expr
exist-expr = [neg-op] path ; path existence or non-existence
Copy link
Collaborator Author

@goessner goessner Nov 3, 2021

Choose a reason for hiding this comment

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

definition of exist-expr might be not necessary with

basic-expr   = [neg-op] path /        ; path existence or non-existence
               [neg-op] paren-expr / 
               relation-expr

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree exist-expr isn't formally necessary, but it gives us a place to highlight the purpose of the syntax and to give it a meaningful name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that was what I was thinking ... agreed, so let it in.

@@ -534,8 +534,7 @@ member value of the input
node corresponding to the member name `"a"`.
The result is again a list of one node: `[{"b":0},{"b":1},{"c":2}]`.

Next, `[*]` selects from any input node which is an array and selects all the elements
of the input node.
Next, `[*]` selects from any input node — either an array or an object — all its elements or members.
Copy link
Member

Choose a reason for hiding this comment

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

You can't select members, only member values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I overlooked, that Glyn's original text was correct, as he was referring to that specific example. So possibly better

Next, `[*]` selects from any input node — an array here — all its elements.

@cabo is right regarding members though.

@@ -1000,7 +997,7 @@ as many times in the node list.
The filter selector has the form `[?<expr>]`. It works via iterating over structured values, i.e. arrays and objects.

~~~~ abnf
filter-selector = "[?" boolean-expr "]"
filter-selector = "[?" S boolean-expr S "]"
Copy link
Member

Choose a reason for hiding this comment

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

So no space between [ and ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no ... might be an extension to the standard making no harm.

; binds more tightly than disjunction

basic-expr = exist-expr / paren-expr / (neg-op paren-expr) / relation-expr
basic-expr = exist-expr /
Copy link
Member

Choose a reason for hiding this comment

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

Style comment: I have tried to indicate ABNF operator precedence using parentheses. Maybe not needed here because of the grouping into lines.

Copy link
Collaborator

@glyn glyn left a comment

Choose a reason for hiding this comment

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

This PR could be merged to make progress. Further changes as indicated by @cabo are probably needed, either now or in a subsequent PR.

@cabo cabo merged commit 2258293 into ietf-wg-jsonpath:main Nov 4, 2021
@cabo
Copy link
Member

cabo commented Nov 4, 2021

Indeed. I'll do a small PR with my points.

cabo added a commit that referenced this pull request Nov 4, 2021
@goessner
Copy link
Collaborator Author

goessner commented Nov 4, 2021

a little late to the party ... thanks

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.

3 participants