Add more set methods #28

gsathya · 2018-05-21T21:06:44Z

Fixes #16

mathiasbynens · 2018-05-21T21:18:11Z

spec/set-prototype-is-disjoint.html

+    1. Else, let _otherSet_ be _iterable_.
+    1. Let _hasCheck_ be ? Get(_otherSet_, "has").
+    1. If IsCallable(_hasCheck_) is *false*, throw a *TypeError* exception.
+    1. For each _e_ that is an element of _entries_, do


Nit: e → entry (no reason to use single-letter variable names here)

Same goes for the other methods

Leaving as such to be consistent with the spec

mathiasbynens · 2018-05-21T21:21:54Z

spec/set-prototype-is-subset.html

+    1. Let _hasCheck_ be ? Get(_otherSet_, "has").
+    1. If IsCallable(_hasCheck_) is *false*, throw a *TypeError* exception.
+    1. For each _e_ that is an element of _entries_, do
+        1. Let _has_ be ? Call(_hasCheck_, _otherSet_, « e »)


Nit: missing .

mathiasbynens · 2018-05-21T21:22:56Z

spec/set-prototype-is-disjoint.html

@@ -0,0 +1,21 @@
+<p>When the `isDisjoint` method is called, the following steps are taken:</p>


Missing “with argument …” — see #27

Same for the other methods

rbuckton · 2018-05-21T21:30:46Z

docs/ecmarkup.js

@@ -119,7 +119,11 @@ Search.prototype.search = function (searchString) {

    for (var i = 0; i < this.biblio.length; i++) {
      var entry = this.biblio[i];
-
+      if (!entry.key) {


Is there a bug for this on https://github.com/bterlson/ecmarkup?

No. Why is this a bug? I'm not sure how ecmarkup generates this output, feel free to file an issue if you think this is incorrect.

rbuckton · 2018-05-21T21:38:46Z

spec/index.html

@@ -39,3 +39,18 @@ <h1>Set.prototype.difference(_iterable_)</h1>
    <h1>Set.prototype.symmetricDifference(_iterable_)</h1>
    <emu-import href="./set-prototype-symmetric-difference.html"></emu-import>
 </emu-clause>
+
+<emu-clause id="Set.prototype.isSubset">
+    <h1>Set.prototype.isSubset(_iterable_)</h1>


The name isSubset seems ambiguous. In a.isSubset(b) are you testing whether: a is a subset of b or b is a subset of a? It could be construed either way. I would recommend a name that better indicates the relationship such as:

a.isSubsetOf(b) - Indicates a is a subset of b

a.hasSubset(b) - Indicates b is a subset of a

rbuckton · 2018-05-21T21:39:56Z

spec/index.html

+</emu-clause>
+
+<emu-clause id="Set.prototype.isSuperset">
+    <h1>Set.prototype.isSuperset(_iterable_)</h1>


As with isSubset, isSuperset seems ambiguous. I would recommend a name that better indicates the relationship such as:

a.isSupersetOf(b) - Indicates a is a superset of b

a.hasSuperset(b) - Indicates b is a superset of a

I agree that isSupersetOf and isSubsetOf are better names.

Ginden · 2018-05-22T09:30:10Z

spec/set-prototype-is-subset.html

+    1. Let _entries_ be the List that is _set_.[[SetData]].
+    1. If Type(_iterable_) is not Object, throw a *TypeError* exception.
+    1. If _iterable_ does not have [[SetData]] internal slot,
+        1. Let _otherSet_ be Construct(%Set%, _iterable_).


Shouldn't we use SpeciesConstructor here?

I don't see any benefits of using a SpeciesConstructor here.

Ginden · 2018-05-22T09:31:27Z

spec/set-prototype-is-superset.html

+
+</emu-alg>
+
+<p>The `length` property of the `isSuperSet` method is *1*.</p>


Inconsistent casing.

Ginden · 2018-05-22T09:32:03Z

spec/index.html

+</emu-clause>
+
+<emu-clause id="Set.prototype.isSuperset">
+    <h1>Set.prototype.isSuperset(_iterable_)</h1>


I agree that isSupersetOf and isSubsetOf are better names.

Ginden · 2018-05-22T09:34:34Z

spec/set-prototype-is-disjoint.html

+    1. Let _entries_ be the List that is _set_.[[SetData]].
+    1. If Type(_iterable_) is not Object, throw a *TypeError* exception.
+    1. If _iterable_ does not have [[SetData]] internal slot,
+        1. Let _otherSet_ be ? Construct(%Set%, _iterable_).


Shouldn't we use SpeciesConstructor?

Don't see any benefit here either

Ginden · 2018-05-22T09:39:02Z

spec/set-prototype-is-subset.html

+    1. Else, let _otherSet_ be _iterable_.
+    1. Let _hasCheck_ be ? Get(_otherSet_, "has").
+    1. If IsCallable(_hasCheck_) is *false*, throw a *TypeError* exception.
+    1. For each _e_ that is an element of _entries_, do


It needs to check for empty entries.

bakkot · 2018-12-13T20:06:27Z

spec/index.html

+    <emu-import href="./set-prototype-is-superset-of.html"></emu-import>
+</emu-clause>
+
+<emu-clause id="Set.prototype.isDisjointWith">


Small note: I believe "is disjoint from" is significantly more common than "is disjoint with". Googling each phrase gives almost an order of magnitude more results for "is disjoint from", and the first result for "is disjoint with" is someone asking if it's correct (whereas the first result for "is disjoint from" is a math article).

Can you raise new issue about this?

per tc39/proposal-set-methods#28, tc39/proposal-set-methods#33

mathiasbynens reviewed May 21, 2018

View reviewed changes

rbuckton reviewed May 21, 2018

View reviewed changes

rbuckton reviewed May 22, 2018

View reviewed changes

Ginden reviewed May 22, 2018

View reviewed changes

gsathya added 3 commits June 10, 2018 10:19

Add more methods

f1eadf7

rename + fix nits

fe0f2fd

update generated spec text

be47b44

gsathya force-pushed the more-helper-methods branch from f469885 to be47b44 Compare June 10, 2018 17:22

Ginden merged commit be47b44 into tc39:master Dec 13, 2018

bakkot reviewed Dec 13, 2018

View reviewed changes

zloirock added a commit to zloirock/core-js that referenced this pull request Dec 18, 2018

add Set#{ isDisjointWith, isSubsetOf, isSupersetOf }

fbb91a3

per tc39/proposal-set-methods#28, tc39/proposal-set-methods#33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more set methods #28

Add more set methods #28

gsathya commented May 21, 2018

mathiasbynens May 21, 2018

mathiasbynens May 21, 2018

gsathya Jun 10, 2018

mathiasbynens May 21, 2018

gsathya Jun 10, 2018

mathiasbynens May 21, 2018

mathiasbynens May 21, 2018

gsathya Jun 10, 2018

rbuckton May 21, 2018

gsathya Jun 10, 2018

rbuckton May 21, 2018

gsathya Jun 10, 2018

rbuckton May 21, 2018

Ginden May 22, 2018

gsathya Jun 10, 2018

Ginden May 22, 2018

gsathya Jun 10, 2018

Ginden May 22, 2018

gsathya Jun 10, 2018

Ginden May 22, 2018

Ginden May 22, 2018

gsathya Jun 10, 2018

Ginden May 22, 2018

gsathya Jun 10, 2018

bakkot Dec 13, 2018

Ginden Dec 13, 2018

		@@ -0,0 +1,21 @@
		<p>When the `isDisjoint` method is called, the following steps are taken:</p>


		</emu-alg>

		<p>The `length` property of the `isSuperSet` method is 1.</p>

Add more set methods #28

Add more set methods #28

Conversation

gsathya commented May 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment