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

Parameterised Lists -> Lists #839

Closed
mnot opened this issue Jul 4, 2019 · 19 comments
Closed

Parameterised Lists -> Lists #839

mnot opened this issue Jul 4, 2019 · 19 comments

Comments

@mnot
Copy link
Member

mnot commented Jul 4, 2019

Discussion on #816 and #797 makes me wonder whether it would be good to refactor Parameterised Lists to be just Lists -- i.e.:

The obvious benefit here is simplification. Another benefit would be future-proofing headers that might need to elaborate on their list items in the future; they can just add new parameters as necessary.

The downsides I can see:

  • As per discussion in Parameterised Identifier -> Parameterised Item? #797, it's no longer simple to map lists to a simple programming language data type, nor to simple JSON. However, I think I could adjust the JSON in the test suite to give us the expressiveness we need here, and I think the wrapper object here is pretty self-explanatory.
  • It would require current specs using Parameterised List to re-specify using List. Not a big deal, I think, but worth checking with folks like @mikewest.
  • A header that wants any list item to fail parsing if it contains parameters will have to say so explicitly -- but as per above, I think that's the right default.

What do folks think -- worth it?

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

@phluid61 @bsdphk @kazuho would also like your thoughts.

@evert
Copy link
Contributor

evert commented Jul 4, 2019

I ran into an issue recently where I wanted to define a new header as a List. I ended up choosing Parameterized List instead. Even though I didn't have any parameters yet, this seemed like a better way to future-proof it.

I'm a proponent of this.

@phluid61
Copy link
Collaborator

phluid61 commented Jul 4, 2019

It's graceful and DRY, but the javascript people probably won't like it. I'm also in a museum in Darwin, a thousand miles from my computer, so I'm not digging very deep right now.

Within a list, is there a difference between an item, and a parameterised item with no parameters? And if so, how are they distinguished in the serialisation?

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

I don't think so -- I think it's just "list items can have parameters."

@annevk any thoughts?

Enjoy the North -- hopefully it's warmer than Melbourne :)

@kazuho
Copy link
Contributor

kazuho commented Jul 4, 2019

My +1 goes to adopting this too. It's simpler, and more future-proof.

Regarding similarity to JSON, it's my understanding that sh-param already requires a not-so-straightforward mapping. Because a JSON object does not have a concept of "a value associated with attributes."

I think there are at least two ways to map an parameterized item to JSON. One is to use an array, like ["value", {"param1": 123, "param2": 456}]. The other is to use a special attribute to represent the public value of the parameterlized item, like {"_": "value", "param1": 123, "param2": 456}. We can use the latter if we need to have distinction between a list and a parameterlized item.

@mikewest
Copy link
Member

mikewest commented Jul 4, 2019

I'm having trouble wrapping my head around the proposal. I think that it means that headers can be more expressive and complex, which sounds great to me! Would you mind putting up an example of what this change would enable?

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

I don't think there's any change in the serialisation -- it's just getting rid of the distinction between List and Parameterised List, so that in headers you define, you just specify List for both use cases.

@mikewest
Copy link
Member

mikewest commented Jul 4, 2019

I don't think there's any change in the serialisation

Then I really don't understand the proposal. :) Going back to the original issue:

All List items would be allowed to have parameters; a "bare" list would just have no parameters

I think this means that list_item1;a=b;c;d=e, list_item2;f=g would be a valid representation of a list with two items, each of which had parameters.

By necessity, List items would still be able to be any item type (or an inner-list, if #816 goes forward), meaning that #797 would be implemented

I think this means that *[binary goes here]*;a=b;c;d=e, 18.2345432;f=g and *[binary goes here]*;a=b;c;d=e, (list_item2 list_item3);f=g would both also be valid.

Is that about right?

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

I think this means that list_item1;a=b;c;d=e, list_item2;f=g would be a valid representation of a list with two items, each of which had parameters.

Yes.

I think this means that *[binary goes here]*;a=b;c;d=e, 18.2345432;f=g and *[binary goes here]*;a=b;c;d=e, (list_item2 list_item3);f=g would both also be valid.

Yes. I'm saying that the serialisation doesn't change because this issue is depending on #816 and #797 to do the work for it; if they get in, it seems like a natural cleanup / consolidation to me.

@mikewest
Copy link
Member

mikewest commented Jul 4, 2019

Got it. 👍 More expressiveness is good.

(list_item2 list_item3);f=g

What about (list1_item1;a=b;c=d list1_item2;e=f);g=h;i=j, (list2_item1;a=b;c=d list2_item2;e=f)?

I assume parameter name (and dictionary item names) wouldn't change? That is, list_item1;(param1 param2)=whatever would still be invalid?

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

No to both of those, at least as I see it currently (I could be convinced, but it seems pretty complex).

@mikewest
Copy link
Member

mikewest commented Jul 4, 2019

Allowing parameterized inner-list items would address the example in https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-10#appendix-B.2. I think it could then be rewritten as:

Example-Thing:
    name="widget",
    cost=89.2,
    descriptions=(foo;url="https://example.net";context=123
                  bar;url="https://example.org";context=456)

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

I'm not against it, but let's see if we can declare victory on this step first :)

@mikewest
Copy link
Member

mikewest commented Jul 4, 2019

Victory it is! I am defining a header whose value is probably(?) a parameterized list literally right now, and will happily change it to a list if/when this lands.

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

OK. I might land the PRs for those two issues and do this, then submit a new draft after @bsdphk has a chance to look over it. I'll try to update the test suite and my implementation before submission to flush out any bugs, but it might have to wait until after (deadline is Monday).

@annevk
Copy link

annevk commented Jul 4, 2019

I think this is fine. We haven't really started looking into JavaScript types for these things yet and that still seems a little early to do as well. And even then we could offer convenience methods that return a lossy array (without parameters) if that turns out to be a common use case.

(And it's indeed very likely you want to go from items to more expressive items, I've come across this a few times in the last couple months alone.)

@mnot
Copy link
Member Author

mnot commented Jul 4, 2019

See PR #841 for a first pass at this.

@mnot
Copy link
Member Author

mnot commented Jul 5, 2019

Also @evert wanted to make sure you saw this, as it's an API change. Thoughts?

@mnot
Copy link
Member Author

mnot commented Jul 7, 2019

Seems like only positive feedback, so I'm going to merge, take another look in situ, and then perhaps get a new draft out. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants