search

ietf-wg-jsonpath / draft-ietf-jsonpath-base

Development of a JSONPath internet draft
https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/
Other
59 stars 20 forks source link

Allow wildcard in list selector #256

Closed glyn closed 1 year ago

glyn commented 2 years ago

Fixes https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/255

gregsdennis commented 2 years ago

I'm open to this, but can we hold this until #201 is resolved, please? I'm really trying to ensure there are no functional changes with it, and merging functional changes while I'm writing a PR are hard to track. I fear something will be missed.

glyn commented 2 years ago

I'm open to this, but can we hold this until #201 is resolved, please? I'm really trying to ensure there are no functional changes with it, and merging functional changes while I'm writing a PR are hard to track. I fear something will be missed.

Ok, I think we need to agree a strategy. I thought that we would pick out any functional changes arising in #201 so that all that remains in a PR for #201 would be editorial changes.

If we agree this strategy, then we need to decide whether #255 (implemented in this PR) is a functional change arising in #201 that we want to adopt. @cabo suggested it. I agree with it. What do others think?

gregsdennis commented 2 years ago

I thought that we would pick out any functional changes arising in #201 so that all that remains in a PR for #201 would be editorial changes.

Yes, this is the strategy.

we need to decide whether #255 (implemented in this PR) is a functional change...

Yes, this is a functional change. Currently, the document disallows * inside what is currently the "list selector." The change is to allow this.

... arising in #201 that we want to adopt.

Perhaps arising from, but not included in #201. #201 is intended to only contain editorial, non-functional changes. This change was suggested by #201, but it was a mistake. I believe it has been reverted from my draft edits so that current functionality is maintained.

glyn commented 2 years ago

This change was suggested by #201, but it was a mistake.

Ok, let's defer merging this for now. We can discuss it separately from #201. Also, the diff is small, so we can easily apply it after #201 if we decide to. I'll turn it into a draft PR for now to avoid it being merged prematurely.

(BTW github seems to be playing up. My comment https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/pull/256#issuecomment-1232524305 took a while to appear and I thought it had gone AWOL, so I posted a similar comment with a more expanded description of the process which I presume will become visible in a few hours. Ho hum...)

glyn commented 1 year ago

This was implemented in PR #258.