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

Use "selector" instead of "matcher" #22

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

glyn
Copy link
Collaborator

@glyn glyn commented Sep 24, 2020

Fixes #20

See the rendered HTML version of this PR.

@glyn glyn self-assigned this Sep 24, 2020
@glyn
Copy link
Collaborator Author

glyn commented Sep 24, 2020

I particularly welcome @gregsdennis's comments since he raised #20.

Copy link
Collaborator

@gregsdennis gregsdennis 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 the matcher -> selector changes are good. I'd just add the word "distinct" in the place indicated.

Reading through, I saw a few other language nuances that could be cleared up. I'll raise them in separate issues for discussion.

Each selector acts on its input list of nodes as follows. For each
node in the list, the selector selects zero or more nodes, each of
which is a descendant of the node or the node itself. The output
list of nodes of a selector is the concatenation of the lists of
Copy link
Collaborator

Choose a reason for hiding this comment

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

"distinct concatenation"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does distinct concatenation mean with duplicates removed? That term is unfamiliar to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct, no duplicates. I think mine currently regards the same value from different locations as distinct from one another, meaning it'll keep both.

It's worth discussing further, probably. Maybe just merge this for now, and I'll open an issue for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I'd like to keep that as a separate issue. It's somewhat related to the placeholder we have in the spec about whether to remove duplicates from unions.

@glyn glyn merged commit bf2098d into ietf-wg-jsonpath:master Sep 24, 2020
@glyn glyn deleted the 20-selector-terminology branch September 24, 2020 12:46
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.

Unify terms
2 participants