w3c / wpub

W3C Web Publications
https://w3c.github.io/wpub/
Other
78 stars 19 forks source link

incorporate webidl definitions into the body of the specification #348

Closed mattgarrish closed 6 years ago

mattgarrish commented 6 years ago

This is the realization of an idea Ivan and I had discussed about how to incorporate the webidl definitions into the body of the specification to reduce the redundancy in having an appendix.

The only minor issue I encountered was with the Contributor class. It seemed odd to have a Creator section begin with a definition using a Contributor sequence. I changed this to CreatorInfo to avoid a dfn collision with the property, but other names welcome. The other issue is that we lacked property definitions to link the members to so I took the existing dl and incorporated it into the infoset section.

@iherman and @HadrienGardeur please have a good look over these changes and let me know if there's anything I've missed, lost, got wrong or otherwise made a mess of.

This PR will also fix the issues raised in #347.


Preview | Diff

iherman commented 6 years ago

Worth noting https://github.com/w3c/respec/issues/1855. If that issue is resolved positively, the IDL Index will "merge" all the partial dictionary definitions into one, which would make it more readable. If that does not happen, we can also generate that appendix by hand at the time of the final publication of the document.

iherman commented 6 years ago

Cc @TzviyaSiegman @wareid @GarthConboy

iherman commented 6 years ago

On a second thought... I wonder whether it is not better to place the WebIDL portions after the "manifest expression" subsections. After all, an author of a WP does not care about the WebIDL, she would only look at the table to know what can and should go into the JSON-LD. The WebIDL is only relevant for implementers, ie, its placement would be slightly de-emphasized by that type of move.

What do people think?

mattgarrish commented 6 years ago

I wonder whether it is not better to place the WebIDL portions after the "manifest expression" subsections.

For the record, I put it after the manifest and infoset introduction as it's sort of the third part in the model. The more practical reason why it ended up there is that we define WebIDL interfaces beginning in the next section, so I thought it might be confusing for people to start encountering the snippets before having any idea exactly why they're there.

iherman commented 6 years ago

For the record, I put it after the manifest and infoset introduction as it's sort of the third part in the model. The more practical reason why it ended up there is that we define WebIDL interfaces beginning in the next section, so I thought it might be confusing for people to start encountering the snippets before having any idea exactly why they're there.

I am not sure I understand. There is a section describing what WebIDL is used for in general, so it sounds ok to have the individual snippets after manifest term tables...

I seem to miss something.:-(

HadrienGardeur commented 6 years ago

I'm not sure how I feel about this. I understand the idea, but it feels IMO confusing to have a mix of IDL and JSON in most sections.

Since the IDL appendix is not that long anyway and will only be relevant for UAs, I don't mind having this separately at the end of our spec.

mattgarrish commented 6 years ago

I am not sure I understand

Ya, probably because I misread your comment. I thought you were asking about moving what is in 3.2 to after the manifest sections in 3.3. Moving the snippets around in the property definitions is doable. I've commonly seen definitions like this introduce the sections, but other ideas are welcome.

mattgarrish commented 6 years ago

Since the IDL appendix is not that long anyway and will only be relevant for UAs, I don't mind having this separately at the end of our spec.

It just feels a bit duplicative to have the WebIDL list definitions that replicate what is generally already being specified in the body, and in most cases have to reference back to the definitions. There's also a bit of redundancy in having both the webidl and the idl index as appendices, as they basically expose the same information.

I suspect most people won't grok the webidl definitions, but they are useful to read where they apply for those who do. If you're looking for the comprehensive reference, the IDL index can now provide that (although the current limitation of not compiling all the partials into a single dictionary is a bit annoying).

iherman commented 6 years ago

as @mattgarrish puts it:

Since the IDL appendix is not that long anyway and will only be relevant for UAs, I don't mind having this separately at the end of our spec.

It just feels a bit duplicative to have the WebIDL list definitions that replicate what is generally already being specified in the body, and in most cases have to reference back to the definitions. There's also a bit of redundancy in having both the webidl and the idl index as appendices, as they basically expose the same information.

Correct on both grounds. The current structure also forces the reader to jump back and forth to understand what the real definition for an IDL term is. Not a good practice on readability ground.

I would agree, on the other hand, with @HadrienGardeur on "IDL [...] will only be relevant for UAs". Hence my feeling now that systematically putting the WebIDL as some sort of an "appendix" for each manifest term that it is meant to represent is a better option. It makes it easier for the user to read through the document, and skip over it if not looking at the text from a UA point of view, while avoiding the major redundancy that we have in the document today.

mattgarrish commented 6 years ago

systematically putting the WebIDL as some sort of an "appendix" for each manifest term that it is meant to represent is a better option.

What to do with the webidl fragments that aren't linked directly to a property? (type, publicationlink, etc.) Also drop them at the end of whatever section they're currently placed in?

mattgarrish commented 6 years ago

I've swapped the definitions to either be the last thing in a section (for those not in section 4 or that don't have manifest sections, like with the language) or to occur just before the example(s). Not sure I like, but I don't care all that strongly, either.

iherman commented 6 years ago

Thanks @mattgarrish, I actually like it:-)

I was looking at the text in 3.2, which says:

To ensure interoperability when exposing the infoset items, this specification defines an abstract representation of the data structures using the Web Interface Definition Language (WebIDL) [webidl-1] which can express the expected names, datatypes, and possible restrictions for each member of the infoset. (A WebIDL representation can be mapped onto ECMAScript, C, or other programming languages.)

To be absolutely precise, the WebIDL represents the manifest expression of the infoset. It may be important to emphasize that; in some cases, as we know, the infoset term maps onto a whole family of manifest terms, and these are all reflected in the WebIDL. Maybe making this clearer in the text above would be better:

... which can express the expected names, datatypes, and possible restrictions for each term in the manifest which, in turn, represent members of the infoset. ...

I am sure you'll find a better way to express this. It also explains why the WebIDL is actually appearing together with the manifest expression and not on the infoset level, so to say.

mattgarrish commented 6 years ago

See if the latest commit capture it.

iherman commented 6 years ago

On 22 Oct 2018, at 02:03, Matt Garrish <notifications@github.com mailto:notifications@github.com> wrote:

See if the latest commit captures it.

I have made a minor change to make it more precise.

If you agree with it I would propose to merge the PR...

mattgarrish commented 6 years ago

for each member of the manifest that expose the infoset.

The manifest can't really expose the infoset, can it? It sort of sounds like the reverse action -- like the manifest is created from the infoset. That's why I had "that is exposed through the infoset", since the manifest isn't directly accessed except by a user agent compiling the infoset. But we're not actually exposing the infoset, so maybe the entire concept is wrong?

Would it maybe be simpler to use "expressed" here:

for each member of the manifest expressed in the infoset

iherman commented 6 years ago

We actually discussed infoset at the F2F:

https://www.w3.org/publishing/groups/publ-wg/Meetings/Minutes/2018/2018-10-22-pwg.html#section2

and one of the points will be that we will have to review the full notion of infoset and probably remove it. This will lead to a major re-write things in this respect...

All that to say that the exact details will disappear later. It is fine to make the change and then we should merge asap. We will have some more editing to do:-)

mattgarrish commented 6 years ago

we will have to review the full notion of infoset and probably remove it

That's pretty much what I asked when the concept of the canonical manifest was introduced. ;) We have a lot of models going on in the document, which probably makes things more complicated than they need to be. But for another day.

I'll fix and merge in a moment.