Re: [csswg-drafts] [css-highlight-api] maplike vs setlike (#5910)

The CSS Working Group just discussed `maplike vs setlike`, and agreed to the following:

* `RESOLVED: Switch Highlights back to a maplike of (name)=> Highlight, add a Note about what happens if you register the same highlight with multiple names`

<details><summary>The full IRC log of that discussion</summary>
&lt;Rossen_> Topic: maplike vs setlike<br>
&lt;Rossen_> github: https://github.com/w3c/csswg-drafts/issues/5910<br>
&lt;TabAtkins> fantasai: A custom highlight is made of three things: the Highlight object (a bunch of ranges), then a priority to know where it stacks, then a name so it's addressable from teh stylesheet<br>
&lt;TabAtkins> s/fantasai/florian/<br>
&lt;TabAtkins> florian: Question is how to group these<br>
&lt;TabAtkins> florian: Could say priority is an attribute of the highlight object<br>
&lt;TabAtkins> florian: But what about the name?<br>
&lt;TabAtkins> florian: One is a Map where key is name and vlaue is Highlight<br>
&lt;TabAtkins> florian: Another is a Set where name is a property of Highlight.<br>
&lt;Rossen_> s/maplike vs setlike/Highlight API collection, maplike vs setlike/<br>
&lt;TabAtkins> florian: If you use a Map, you coudl potentially register the same highlight multiple times under multiple names.<br>
&lt;TabAtkins> florian: Has some odd ergonomics for user.<br>
&lt;TabAtkins> florian: You style ::custom-spelling and ::custom-grammar, but they have the same priority, it's got some odd stacking.<br>
&lt;TabAtkins> florian: Not *hard* to explain or implement, just unintuitive. We consider it a footgun.<br>
&lt;TabAtkins> florian: So we decided to make it impossible to set up the same highlight multiple times.<br>
&lt;hober> q+<br>
&lt;TabAtkins> florian: So we switched to a Set that throws if you register multiple times. We could also have a Set that just ignores if you set multiple.<br>
&lt;TabAtkins> florian: Or a Map that throws if you register multiple.<br>
&lt;TabAtkins> florian: Or we could just ignore it all and let it happen.<br>
&lt;leaverou_> q+<br>
&lt;TabAtkins> florian: Currently the spec has it as a Set where name is an attribute, and it throws if you register the same thing twice.<br>
&lt;TabAtkins> florian: But that's unusual.<br>
&lt;TabAtkins> florian: Simplest is a Map that doesn't throw, but that might not make sense. But maybe that's fine.<br>
&lt;TabAtkins> hober: This came up in a TAG review<br>
&lt;Rossen_> ack hober<br>
&lt;TabAtkins> hober: been several months so convo is swapped out by now...<br>
&lt;TabAtkins> hober: But i remember being surprised about, it would be common to write code that says "have I registered this yet? No? then add it"<br>
&lt;TabAtkins> hober: Might have a few libraries that use highlights for various purposes, and they don't want to clobber themselves<br>
&lt;TabAtkins> hober: With a Map that's super easy<br>
&lt;TabAtkins> hober: just check the key<br>
&lt;TabAtkins> hober: With a set you have to iterate<br>
&lt;TabAtkins> hober: We thought that's weird, we expected checking to see if it's registered to be a common op<br>
&lt;TabAtkins> hober: I think we saw the ergonomic downside as more of dev inconvenience than the dev registering the same thing multiple times<br>
&lt;TabAtkins> hober: So I think we're inclined to just let the dev hurt themselves if they want to register things twice<br>
&lt;TabAtkins> hober: Not a hill we want to die on, just want common operations to be easy.<br>
&lt;sanketj> q+<br>
&lt;TabAtkins> florian: With time that has passed since original decision, sounds good to me<br>
&lt;Rossen_> ack leaverou_<br>
&lt;TabAtkins> leaverou_: Agree with hober, she said most of what I wanted to<br>
&lt;TabAtkins> leaverou_: I think using try/catch every time you register a highlight isn't ideal<br>
&lt;TabAtkins> leaverou_: I think there might even be use-cases for doing this - providing aliases for the same highlight<br>
&lt;TabAtkins> leaverou_: Sanket pointed out the problem is the priority (can't set it in that case), but that sounds like priority is associated with the name rather than the Highlight... maybe it's misplaced?<br>
&lt;TabAtkins> florian: I thougth about that, but then how would you set it?<br>
&lt;TabAtkins> sanketj: Design I had in mind originally was every Highlight has one name and one priority, and that's reflected in the spec we have today.<br>
&lt;TabAtkins> sanketj: You could say the prio is associated with the name rather than the highlight, but you'd need a different data structure<br>
&lt;TabAtkins> Rossen_: What's your position on the change to use Map?<br>
&lt;iank_> sanketj: likely want an options arg for the ctor, e.g. "new Highlight({name: 'hi', priority: 2})"<br>
&lt;TabAtkins> Yeah, it'd take a sequence or options object instead, i guess<br>
&lt;iank_> can't it be a maplike with an additional convenince function?<br>
&lt;TabAtkins> sanketj: hober and leaverou_'s argumetns make sense. And I think associating prio with highlight is convenient; I'd prefer not to change that if there's not a big reason to<br>
&lt;TabAtkins> sanketj: If we do allow a highlight to have multiple names, what's the messaging around it?<br>
&lt;TabAtkins> sanketj: Lea mentioned a use-case; I always thought about this as making a separate Highlight object, then they can prio against each other properly<br>
&lt;TabAtkins> sanketj: Sounds like you were suggesting having the same highlight be prio'd two different ways, which might be more complicated<br>
&lt;Rossen_> q?<br>
&lt;TabAtkins> florian: I think the proposal is to just do it the naive way - same highligh under two names would share prio<br>
&lt;TabAtkins> florian: So painting order would use the fallback of registration order<br>
&lt;TabAtkins> florian: A bit limiting, but it's not there *because* of the use-case, it's just the natural fallout.<br>
&lt;TabAtkins> florian: And if people find ways to make it nice for them, sure.<br>
&lt;leaverou_> +1 to florian<br>
&lt;TabAtkins> florian: Probably it's a footgun, but possibly there's good things to come out of it<br>
&lt;iank_> e.g. ```interface Highlights { readonly maplike&lt;string, Highlight>; addHighlight(Highlight or HightlightDictionary); }```<br>
&lt;TabAtkins> sanketj: Do we want something in the spec warning against it?<br>
&lt;leaverou_> Yes, this would need to be a note in the spec<br>
&lt;TabAtkins> sanketj: Seems could be unexpected results<br>
&lt;TabAtkins> florian: Don't think we should have a normative thing, but a Note saying this would be odd if you did it woudl be approriate<br>
&lt;TabAtkins> leaverou_: agreed<br>
&lt;TabAtkins> sanketj: We could just document the case from the issue<br>
&lt;TabAtkins> sanketj: Seems fine<br>
&lt;TabAtkins> Rossen_: Sounds like we're approaching a reoslution?<br>
&lt;TabAtkins> iank_: I left some IRC comments<br>
&lt;TabAtkins> iank_: You don't need to strictly make it a setlike - could add convenience functions<br>
&lt;TabAtkins> iank_: Dunno if this changes the discussion particularly<br>
&lt;TabAtkins> florian: Not sure what's being proposed to help here<br>
&lt;TabAtkins> iank_: could make it a readonly maplike, then provide an add() that takes a Highlight, plus a remove()<br>
&lt;TabAtkins> florian: Does that bring us back to what we have today?<br>
&lt;TabAtkins> sanketj: I think the impl is that the name would live on the highlight, and also be exposed as a key on the readonly maplike<br>
&lt;TabAtkins> TabAtkins: I don't think the3re's enough worth innovating in data structures, versus just letting authors do this<br>
&lt;TabAtkins> porposed resolution: Switch Highlights back to a maplike of (name)=> Highlight, add a Note about what happens if you register the same highlight with multiple names<br>
&lt;TabAtkins> leaverou_: Just wnat to make sure that if you register to an existing key, it doesn't throw - that was mentioned in the ear4ly options<br>
&lt;TabAtkins> florian: Right, normal Map behavior<br>
&lt;TabAtkins> RESOLVED: Switch Highlights back to a maplike of (name)=> Highlight, add a Note about what happens if you register the same highlight with multiple names<br>
</details>


-- 
GitHub Notification of comment by css-meeting-bot
Please view or discuss this issue at https://github.com/w3c/csswg-drafts/issues/5910#issuecomment-816245337 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Thursday, 8 April 2021 21:48:34 UTC