Re: [ReSpec] release 23.2.0

On Thu, Oct 4, 2018 at 7:07 PM Marcos Caceres <mcaceres@mozilla.com> wrote:
> Hi Tab,
> > On Oct 5, 2018, at 4:55 AM, Tab Atkins Jr. <jackalmage@gmail.com> wrote:
> >
> > Hm, any particular reason you diverged from Bikeshed's syntax so
> > significantly? All of these are the exact same syntax in Bikeshed:
> >
> > * {{PaymentRequest/id}}
> > * {{PaymentRequest/show()}}
> > * {{PayerErrors/"phone"}} (or just {{PayerErrors/phone}}, both work).
>
> We wanted something that was more “WYSIWYG”. Otherwise, for instance, it’s unclear what {{PayerErrors/phone}} is (an attribute or a member?) without having to open up the whole spec and having a look. Thus, this is helpful (for me) when I’m reviewing PRs, because it’s immediately clear that PayerErrors[“phone”] is talking about a dictionary member.

I mean, that's kinda the point - because of how IDL namespacing works,
specifying the precise type is redundant. All of the top-level types
(interface, dictionary, etc) occupy a shared namespace, so they must
be unique; all of the sub-types, once you specify what top-level type
they're "for", also share a namespace underneath that for-value, so
they must be unique. Knowing the actual type doesn't give you
anything.

If it's important to know that {{PayerErrors/phone}} is an attribute
vs a member, then the reviewer, to be able to review that, must
already know whether PayerErrors is an interface or a dictionary; the
syntax distinction is unimportant. If the reviewer *doesn't care*
whether it's an attribute or member, then the syntax distinction is
unimportant here too.

(It also seems odd to make the distinction here, on the
attribute/member level, rather than at the interface/dictionary level.
{{{PayerErrors}}} doesn't tell you whether it's an interface or a
dictionary - why isn't it helpful for reviewers to provide that
information via syntax here?)

> But I agree that it means that Editors need to know the for Dictionary types, they need to write things `Like[“This”]` (but see below… Foo.bar also works for dictionaries).

If, as you say below, `{{{Dict.member}}}` works, then presumably
`{{{Interface["attribute"]}}}` works too? So then the distinction is
purely convention, right, and reviewers can't really depend on it?

> > (Because all of them, when qualified by the interface/etc they belong
> > to, occupy shared namespaces, so there's no need to further
> > distinguish them in syntax.)
>
> I agree - but they way ReSpec works is a little different at the moment: We do inline processing of "{{{ }}}” things before we do IDL parsing (at least, today).
>
> That means we don’t have an IDL AST at that point on which to do the lookups. We will eventually change this, but right now, this works well for most common cases.

Bikeshed works the same way; it processes all `{{foo}}` links into `<a
data-link-type=idl>foo</a>`, which is a link-supertype that covers
*all* of the IDL types, and then only later, when I have parsed the
spec, does it try to actually figure out where it needs to point to.

> > It feels like adding more specific syntaxes to these would make it
> > slightly harder to remember?
>
> I don’t disagree. But it lightens the cognitive load when doing reviews - because “what you see is what you get”: attribute, method(), [“member”], and [[iternal_slot]]. Irrespective {{PayerErrors/phone}} will be converted into markup that renders as `PayerErrors[“phone”]`, so why not just allow it?

Ah, in Bikeshed, {{PayerErrors/phone}} does *not* render into that; by
default, the link's text content will just be `phone`. Rendering to
`PayerErrors["phone"]` is odd, because that expression doesn't mean
what it looks like in JS - attributes/methods aren't stored on the
class object, they're on the prototype object. Dictionaries don't even
have corresponding objects. And it's not a meaningful expression in
IDL either.  (People do sometimes shorthand `Class.attr` to refer to
the attr for instances of that class, but another common convention is
to use #, like `Class#attr`, to indicate a proto-lookup rather than a
literal object-lookup.)

~TJ

Received on Friday, 5 October 2018 18:33:46 UTC