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

Align (and simplify) terminology. #72

Merged
merged 5 commits into from
Mar 23, 2021
Merged

Align (and simplify) terminology. #72

merged 5 commits into from
Mar 23, 2021

Conversation

cabo
Copy link
Member

@cabo cabo commented Mar 13, 2021

I created a pull request so we have something concrete to discuss to close #66.

draft-ietf-jsonpath-base.md Outdated Show resolved Hide resolved
English.

Co-authored-by: Glyn Normington <[email protected]>
draft-ietf-jsonpath-base.md Outdated Show resolved Hide resolved
Copy link
Contributor

@timbray timbray left a comment

Choose a reason for hiding this comment

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

I think in an ideal world, it would be good to use only and exactly the 8259 terminology. "Value", "JSON Text", "Array", "Object". The latest draft isn't fresh enough in my mind to have an opinion as to whether this is possible.

@cabo cabo mentioned this pull request Mar 17, 2021
draft-ietf-jsonpath-base.md Show resolved Hide resolved
draft-ietf-jsonpath-base.md Outdated Show resolved Hide resolved
draft-ietf-jsonpath-base.md Show resolved Hide resolved
Query:
: Short name for JSONPath expression.

Argument:
: Short name for the JSON data item a JSONPath expression is applied to.

Nodelist:
Copy link
Collaborator

@gregsdennis gregsdennis Mar 17, 2021

Choose a reason for hiding this comment

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

Please see my comment. Also @danielaparker's comment regarding "JSON in, JSON out."

Copy link
Member Author

Choose a reason for hiding this comment

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

I have reread both comments now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, so my point of having you reread those comments was to illustrate my discomfort with the "nodelist" concept. You have neither refuted those comments nor changed the PR, so this remains unresolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Some discussion about nodelist has happened. Right now, the discussion in this WG is a bit fragmented; I'll try to dig this out when I have time to search for it. Should have pointed to it in the first place...

Copy link

@danielaparker danielaparker Mar 18, 2021

Choose a reason for hiding this comment

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

Just to note, I share with @gregsdennis some discomfort with the "nodelist" concept, and am skeptical that it's needed to describe JSONPath. I don't think the idea of a "nodelist" is required to describe the distinguishing feature of JSONPath, that it returns a collection of values (or references) to items that already exist in the original JSON document. We have from the very beginning the notion that "the return value of jsonPath is an array, which is also a valid JSON structure. So you might want to apply jsonPath to the resulting structure". Practically all implementations support something like:

JSON = jsonPath(JSON, path, value option)    

or

JSON = jsonPath(JSON, path, path option)

With duplicates allowed, which is the case for legacy implementations, the computational model is simply to collect values (or their associated paths). Where is the role for "nodelists"?

With no duplicates, which is the case for some modern implementations, the computational model is to compute both values and their associated (normalized) paths, and to keep path-value pairs with unique paths. Again, where is the role for "nodelists"?

In XPath, with it's vastly more complicated data model, I understand the need for "nodelist". But I don't understand why we need to take this XPath idea and transpose it to JSONPath. It seems to me to be an unnecessary concept for describing JSONPath.

I would also suggest that the draft isn't clear about what it means by "node". In a discussion in the terminology issue, @glyn suggested that if I wasn't familiar with trees and nodes, I could consult Wikipedia. But what's important is not what I understand or don't understand, what's important is what the draft says. In XPath specifications, the meaning of "node" is crystal clear, in the draft, it is not. I think that lack of clarity is somewhat related to the problem of making an argument for the "nodelist" concept that readers such as myself might find convincing.

I think there should be an issue raised for "nodelist, and the justification thereof". Perhaps @cabo could start one?

Copy link
Member Author

Choose a reason for hiding this comment

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

(I moved nodelist over to a new PR that I'm preparing.)

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.

I left a comment or two, but am generally comfortable with these changes.

@cabo cabo mentioned this pull request Mar 22, 2021
@goessner goessner merged commit 56dc1c5 into master Mar 23, 2021
@cabo cabo deleted the terminology branch March 16, 2023 19:38
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.

Clarification on Terminology
6 participants