From 37b10a0b1f084c9084bb41fd61729e7d754e6183 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Sat, 22 Jun 2024 14:56:43 -0400 Subject: [PATCH 01/17] x/net/html: add Node.All() and Node.ChildNodes() Fixes #62113 --- html/example_test.go | 10 +++--- html/iter.go | 51 ++++++++++++++++++++++++++++++ html/iter_test.go | 75 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 html/iter.go create mode 100644 html/iter_test.go diff --git a/html/example_test.go b/html/example_test.go index 0b06ed773..e9101fd55 100644 --- a/html/example_test.go +++ b/html/example_test.go @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build goexperiment.rangefunc + // This example demonstrates parsing HTML data and walking the resulting tree. package html_test @@ -19,8 +21,7 @@ func ExampleParse() { if err != nil { log.Fatal(err) } - var f func(*html.Node) - f = func(n *html.Node) { + for n := range doc.All() { if n.Type == html.ElementNode && n.Data == "a" { for _, a := range n.Attr { if a.Key == "href" { @@ -29,11 +30,8 @@ func ExampleParse() { } } } - for c := n.FirstChild; c != nil; c = c.NextSibling { - f(c) - } } - f(doc) + // Output: // foo // /bar/baz diff --git a/html/iter.go b/html/iter.go new file mode 100644 index 000000000..174f7f376 --- /dev/null +++ b/html/iter.go @@ -0,0 +1,51 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build goexperiment.rangefunc + +package html + +import "iter" + +// ChildNodes returns a sequence yielding the immediate children of n. +// +// Mutating a Node while iterating through its ChildNodes may have unexpected results. +func (n *Node) ChildNodes() iter.Seq[*Node] { + return func(yield func(*Node) bool) { + if n == nil { + return + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + if !yield(c) { + return + } + } + } + +} + +func (n *Node) all(yield func(*Node) bool) bool { + if n == nil { + return true + } + if !yield(n) { + return false + } + + for c := range n.ChildNodes() { + if !c.all(yield) { + return false + } + } + return true +} + +// All returns a sequence yielding all descendents of n in depth-first pre-order. +// +// Mutating a Node while iterating through it or its descendents may have unexpected results. +func (n *Node) All() iter.Seq[*Node] { + return func(yield func(*Node) bool) { + n.all(yield) + } +} diff --git a/html/iter_test.go b/html/iter_test.go new file mode 100644 index 000000000..d8423188d --- /dev/null +++ b/html/iter_test.go @@ -0,0 +1,75 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build goexperiment.rangefunc + +package html + +import ( + "strings" + "testing" +) + +func TestNode_ChildNodes(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"", ""}, + {"", ""}, + {"", "b"}, + {"b", "b"}, + {"", "b"}, + {"bd", "b c d"}, + {"be", "b c e"}, + {"dfgj", "b g h i"}, + } + for _, test := range tests { + doc, err := Parse(strings.NewReader(test.in)) + if err != nil { + t.Fatal(err) + } + // Drill to test.in + n := doc.FirstChild.FirstChild.NextSibling.FirstChild + var results []string + for c := range n.ChildNodes() { + results = append(results, c.Data) + } + if got := strings.Join(results, " "); got != test.want { + t.Errorf("unexpected children yielded by ChildNodes; want: %q got: %q", test.want, got) + } + } +} + +func TestNode_All(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"", ""}, + {"", "a"}, + {"", "a b"}, + {"b", "a b"}, + {"", "a b"}, + {"bd", "a b c d"}, + {"be", "a b c d e"}, + {"dfgj", "a b c d e f g h i j"}, + } + for _, test := range tests { + doc, err := Parse(strings.NewReader(test.in)) + if err != nil { + t.Fatal(err) + } + // Drill to test.in + n := doc.FirstChild.FirstChild.NextSibling.FirstChild + var results []string + for c := range n.All() { + results = append(results, c.Data) + } + if got := strings.Join(results, " "); got != test.want { + t.Errorf("unexpected children yielded by All; want: %q got: %q", + test.want, got) + } + } +} From b11ed9ad60b30d8b7f2d4e62118b8f1c33eee3a9 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Sat, 22 Jun 2024 15:27:42 -0400 Subject: [PATCH 02/17] add Node.Parents --- go.mod | 2 +- html/iter.go | 11 +++++++++++ html/iter_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 77935d3f2..a402a9b43 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module golang.org/x/net -go 1.18 +go 1.22 require ( golang.org/x/crypto v0.28.0 diff --git a/html/iter.go b/html/iter.go index 174f7f376..f025f46b4 100644 --- a/html/iter.go +++ b/html/iter.go @@ -49,3 +49,14 @@ func (n *Node) All() iter.Seq[*Node] { n.all(yield) } } + +// Parents returns an sequence yielding the node and its parents. +func (n *Node) Parents() iter.Seq[*Node] { + return func(yield func(*Node) bool) { + for p := n; p != nil; p = p.Parent { + if !yield(p) { + return + } + } + } +} diff --git a/html/iter_test.go b/html/iter_test.go index d8423188d..ac25ac7ae 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -73,3 +73,36 @@ func TestNode_All(t *testing.T) { } } } + +func TestNode_Parents(t *testing.T) { + testParents(t, nil, 0) + for size := range 100 { + n := buildChain(size) + testParents(t, n, size+1) + } +} + +func testParents(t *testing.T, n *Node, wantSize int) { + nParents := 0 + for _ = range n.Parents() { + nParents++ + } + if nParents != wantSize { + t.Errorf("unexpected number of Parents; want %d got: %d", wantSize, nParents) + } +} + +func buildChain(size int) *Node { + descendent := &Node{ + Type: ElementNode, + } + current := descendent + for range size { + parent := &Node{ + Type: ElementNode, + } + parent.AppendChild(current) + current = parent + } + return descendent +} From e25f81f14615857f42b4c3b67a713f90bd9a21fc Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Sat, 22 Jun 2024 15:29:35 -0400 Subject: [PATCH 03/17] add warning to Node.Parents --- html/iter.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/html/iter.go b/html/iter.go index f025f46b4..c6ce86d38 100644 --- a/html/iter.go +++ b/html/iter.go @@ -51,6 +51,8 @@ func (n *Node) All() iter.Seq[*Node] { } // Parents returns an sequence yielding the node and its parents. +// +// Mutating a Node while iterating through it or its parents may have unexpected results. func (n *Node) Parents() iter.Seq[*Node] { return func(yield func(*Node) bool) { for p := n; p != nil; p = p.Parent { From 9a8f47f7c6a9ec4aff6df636e117b20263ea394e Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Sat, 22 Jun 2024 15:31:08 -0400 Subject: [PATCH 04/17] ExampleParse: Use atom.A --- html/example_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/html/example_test.go b/html/example_test.go index e9101fd55..5013509c1 100644 --- a/html/example_test.go +++ b/html/example_test.go @@ -13,6 +13,7 @@ import ( "strings" "golang.org/x/net/html" + "golang.org/x/net/html/atom" ) func ExampleParse() { @@ -22,7 +23,7 @@ func ExampleParse() { log.Fatal(err) } for n := range doc.All() { - if n.Type == html.ElementNode && n.Data == "a" { + if n.Type == html.ElementNode && n.DataAtom == atom.A { for _, a := range n.Attr { if a.Key == "href" { fmt.Println(a.Val) From 036f75bdfc2f284a41e22af3db1fc2856a592d52 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Sat, 22 Jun 2024 16:11:31 -0400 Subject: [PATCH 05/17] miscellaneous --- html/iter.go | 6 +++--- html/iter_test.go | 26 +++++++++++--------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/html/iter.go b/html/iter.go index c6ce86d38..f324e59b9 100644 --- a/html/iter.go +++ b/html/iter.go @@ -26,9 +26,6 @@ func (n *Node) ChildNodes() iter.Seq[*Node] { } func (n *Node) all(yield func(*Node) bool) bool { - if n == nil { - return true - } if !yield(n) { return false } @@ -46,6 +43,9 @@ func (n *Node) all(yield func(*Node) bool) bool { // Mutating a Node while iterating through it or its descendents may have unexpected results. func (n *Node) All() iter.Seq[*Node] { return func(yield func(*Node) bool) { + if n == nil { + return + } n.all(yield) } } diff --git a/html/iter_test.go b/html/iter_test.go index ac25ac7ae..d6e2f871b 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -82,6 +82,17 @@ func TestNode_Parents(t *testing.T) { } } +func buildChain(size int) *Node { + descendent := new(Node) + current := descendent + for range size { + parent := new(Node) + parent.AppendChild(current) + current = parent + } + return descendent +} + func testParents(t *testing.T, n *Node, wantSize int) { nParents := 0 for _ = range n.Parents() { @@ -91,18 +102,3 @@ func testParents(t *testing.T, n *Node, wantSize int) { t.Errorf("unexpected number of Parents; want %d got: %d", wantSize, nParents) } } - -func buildChain(size int) *Node { - descendent := &Node{ - Type: ElementNode, - } - current := descendent - for range size { - parent := &Node{ - Type: ElementNode, - } - parent.AppendChild(current) - current = parent - } - return descendent -} From 4eaadbd5683c904f9689151cac279a5dff327c3b Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Thu, 25 Jul 2024 09:15:07 -0400 Subject: [PATCH 06/17] Fix build tag --- go.mod | 2 +- html/iter.go | 2 +- html/iter_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a402a9b43..aaa261f5b 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module golang.org/x/net -go 1.22 +go 1.23 require ( golang.org/x/crypto v0.28.0 diff --git a/html/iter.go b/html/iter.go index f324e59b9..faa290c09 100644 --- a/html/iter.go +++ b/html/iter.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build goexperiment.rangefunc +//go:build go1.23 package html diff --git a/html/iter_test.go b/html/iter_test.go index d6e2f871b..310a5fe4e 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build goexperiment.rangefunc +//go:build go1.23 package html From 3db76ff4681653b7a48e656fa91f187080b3893a Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Thu, 25 Jul 2024 09:15:19 -0400 Subject: [PATCH 07/17] Nicer buildChain --- html/iter_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/html/iter_test.go b/html/iter_test.go index 310a5fe4e..14e572321 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -83,14 +83,13 @@ func TestNode_Parents(t *testing.T) { } func buildChain(size int) *Node { - descendent := new(Node) - current := descendent + child := new(Node) for range size { - parent := new(Node) - parent.AppendChild(current) - current = parent + parent := child + child = new(Node) + parent.AppendChild(child) } - return descendent + return child } func testParents(t *testing.T, n *Node, wantSize int) { From 1d4925f67b11266d625465b7dd5fec65e2dbf78b Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Thu, 25 Jul 2024 09:19:57 -0400 Subject: [PATCH 08/17] Doc: Fix doc comment --- html/doc.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/html/doc.go b/html/doc.go index 3a7e5ab17..daf78f8c5 100644 --- a/html/doc.go +++ b/html/doc.go @@ -78,16 +78,11 @@ example, to process each anchor node in depth-first order: if err != nil { // ... } - var f func(*html.Node) - f = func(n *html.Node) { + for n := range doc.All() { if n.Type == html.ElementNode && n.Data == "a" { // Do something with n... } - for c := n.FirstChild; c != nil; c = c.NextSibling { - f(c) - } } - f(doc) The relevant specifications include: https://html.spec.whatwg.org/multipage/syntax.html and From 2dbb5240dd506101ee54af9792f320e103b05240 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Thu, 25 Jul 2024 09:21:59 -0400 Subject: [PATCH 09/17] Fix another go build --- html/example_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/html/example_test.go b/html/example_test.go index 5013509c1..eda9cb894 100644 --- a/html/example_test.go +++ b/html/example_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build goexperiment.rangefunc +//go:build go1.23 // This example demonstrates parsing HTML data and walking the resulting tree. package html_test From c98fd06c26082eb974fffbe2b8039c74fd5855c6 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Thu, 25 Jul 2024 09:29:39 -0400 Subject: [PATCH 10/17] iter: Better reading order --- html/iter.go | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/html/iter.go b/html/iter.go index faa290c09..f282fd00c 100644 --- a/html/iter.go +++ b/html/iter.go @@ -8,9 +8,22 @@ package html import "iter" +// Parents returns an sequence yielding the node and its parents. +// +// Mutating a Node or its parents while iterating may have unexpected results. +func (n *Node) Parents() iter.Seq[*Node] { + return func(yield func(*Node) bool) { + for p := n; p != nil; p = p.Parent { + if !yield(p) { + return + } + } + } +} + // ChildNodes returns a sequence yielding the immediate children of n. // -// Mutating a Node while iterating through its ChildNodes may have unexpected results. +// Mutating a Node or its ChildNodes while iterating may have unexpected results. func (n *Node) ChildNodes() iter.Seq[*Node] { return func(yield func(*Node) bool) { if n == nil { @@ -25,22 +38,9 @@ func (n *Node) ChildNodes() iter.Seq[*Node] { } -func (n *Node) all(yield func(*Node) bool) bool { - if !yield(n) { - return false - } - - for c := range n.ChildNodes() { - if !c.all(yield) { - return false - } - } - return true -} - // All returns a sequence yielding all descendents of n in depth-first pre-order. // -// Mutating a Node while iterating through it or its descendents may have unexpected results. +// Mutating a Node or its descendents while iterating may have unexpected results. func (n *Node) All() iter.Seq[*Node] { return func(yield func(*Node) bool) { if n == nil { @@ -50,15 +50,15 @@ func (n *Node) All() iter.Seq[*Node] { } } -// Parents returns an sequence yielding the node and its parents. -// -// Mutating a Node while iterating through it or its parents may have unexpected results. -func (n *Node) Parents() iter.Seq[*Node] { - return func(yield func(*Node) bool) { - for p := n; p != nil; p = p.Parent { - if !yield(p) { - return - } +func (n *Node) all(yield func(*Node) bool) bool { + if !yield(n) { + return false + } + + for c := range n.ChildNodes() { + if !c.all(yield) { + return false } } + return true } From 6246224eba010336aeaf0f8925d8dc162673f6a5 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Wed, 28 Aug 2024 20:36:16 -0400 Subject: [PATCH 11/17] Revert go.mod change --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index aaa261f5b..77935d3f2 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module golang.org/x/net -go 1.23 +go 1.18 require ( golang.org/x/crypto v0.28.0 From 239d691b799da8a43e4494e6687071c3cbcdd783 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Wed, 28 Aug 2024 21:12:59 -0400 Subject: [PATCH 12/17] Make changes based on feedback Incorporates https://go-review.googlesource.com/c/net/+/606595 --- html/example_test.go | 2 +- html/iter.go | 60 ++++++++++++++++++++++---------------------- html/iter_test.go | 42 +++++++++++++------------------ html/node.go | 4 +++ 4 files changed, 53 insertions(+), 55 deletions(-) diff --git a/html/example_test.go b/html/example_test.go index eda9cb894..830f0b27a 100644 --- a/html/example_test.go +++ b/html/example_test.go @@ -22,7 +22,7 @@ func ExampleParse() { if err != nil { log.Fatal(err) } - for n := range doc.All() { + for n := range doc.Descendants() { if n.Type == html.ElementNode && n.DataAtom == atom.A { for _, a := range n.Attr { if a.Key == "href" { diff --git a/html/iter.go b/html/iter.go index f282fd00c..59b96649e 100644 --- a/html/iter.go +++ b/html/iter.go @@ -8,55 +8,55 @@ package html import "iter" -// Parents returns an sequence yielding the node and its parents. +// Ancestors returns an iterator over the ancestors of n, starting with n.Parent. +// +// Example: +// +// for ancestor := range n.Ancestors() { ... } // // Mutating a Node or its parents while iterating may have unexpected results. -func (n *Node) Parents() iter.Seq[*Node] { +func (n *Node) Ancestors() iter.Seq[*Node] { + _ = n.Parent // eager nil check + return func(yield func(*Node) bool) { - for p := n; p != nil; p = p.Parent { - if !yield(p) { - return - } + for p := n.Parent; p != nil && yield(p); p = p.Parent { } } } -// ChildNodes returns a sequence yielding the immediate children of n. +// Children returns an iterator over the immediate children of n, +// starting with n.FirstChild. // -// Mutating a Node or its ChildNodes while iterating may have unexpected results. -func (n *Node) ChildNodes() iter.Seq[*Node] { +// Example: +// +// for child := range n.Children() { ... } +func (n *Node) Children() iter.Seq[*Node] { + _ = n.FirstChild // eager nil check + return func(yield func(*Node) bool) { - if n == nil { - return - } - for c := n.FirstChild; c != nil; c = c.NextSibling { - if !yield(c) { - return - } + for c := n.FirstChild; c != nil && yield(c); c = c.NextSibling { } } } -// All returns a sequence yielding all descendents of n in depth-first pre-order. +// Descendants returns an iterator over all nodes recursively beneath +// n, excluding n itself. Nodes are visited in depth-first preorder. // -// Mutating a Node or its descendents while iterating may have unexpected results. -func (n *Node) All() iter.Seq[*Node] { +// Example: +// +// for desc := range n.Descendants() { ... } +func (n *Node) Descendants() iter.Seq[*Node] { + _ = n.FirstChild // eager nil check + return func(yield func(*Node) bool) { - if n == nil { - return - } - n.all(yield) + _ = n.descendants(yield) } } -func (n *Node) all(yield func(*Node) bool) bool { - if !yield(n) { - return false - } - - for c := range n.ChildNodes() { - if !c.all(yield) { +func (n *Node) descendants(yield func(*Node) bool) bool { + for c := range n.Children() { + if !yield(c) || !c.descendants(yield) { return false } } diff --git a/html/iter_test.go b/html/iter_test.go index 14e572321..f3b019518 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -11,12 +11,11 @@ import ( "testing" ) -func TestNode_ChildNodes(t *testing.T) { +func TestNode_Children(t *testing.T) { tests := []struct { in string want string }{ - {"", ""}, {"", ""}, {"", "b"}, {"b", "b"}, @@ -30,19 +29,19 @@ func TestNode_ChildNodes(t *testing.T) { if err != nil { t.Fatal(err) } - // Drill to test.in + // Drill to n := doc.FirstChild.FirstChild.NextSibling.FirstChild var results []string - for c := range n.ChildNodes() { + for c := range n.Children() { results = append(results, c.Data) } if got := strings.Join(results, " "); got != test.want { - t.Errorf("unexpected children yielded by ChildNodes; want: %q got: %q", test.want, got) + t.Errorf("unexpected children yielded by Children; want: %q got: %q", test.want, got) } } } -func TestNode_All(t *testing.T) { +func TestNode_Descendants(t *testing.T) { tests := []struct { in string want string @@ -61,24 +60,29 @@ func TestNode_All(t *testing.T) { if err != nil { t.Fatal(err) } - // Drill to test.in - n := doc.FirstChild.FirstChild.NextSibling.FirstChild + // Drill to + n := doc.FirstChild.FirstChild.NextSibling var results []string - for c := range n.All() { + for c := range n.Descendants() { results = append(results, c.Data) } if got := strings.Join(results, " "); got != test.want { - t.Errorf("unexpected children yielded by All; want: %q got: %q", + t.Errorf("unexpected children yielded by Descendants; want: %q got: %q", test.want, got) } } } -func TestNode_Parents(t *testing.T) { - testParents(t, nil, 0) - for size := range 100 { +func TestNode_Ancestors(t *testing.T) { + for _, size := range []int{0, 1, 2, 10, 100, 10_000} { n := buildChain(size) - testParents(t, n, size+1) + nParents := 0 + for _ = range n.Ancestors() { + nParents++ + } + if nParents != size { + t.Errorf("unexpected number of Ancestors; want %d got: %d", size, nParents) + } } } @@ -91,13 +95,3 @@ func buildChain(size int) *Node { } return child } - -func testParents(t *testing.T, n *Node, wantSize int) { - nParents := 0 - for _ = range n.Parents() { - nParents++ - } - if nParents != wantSize { - t.Errorf("unexpected number of Parents; want %d got: %d", wantSize, nParents) - } -} diff --git a/html/node.go b/html/node.go index 1350eef22..77741a195 100644 --- a/html/node.go +++ b/html/node.go @@ -38,6 +38,10 @@ var scopeMarker = Node{Type: scopeMarkerNode} // that it looks like "a Date: Wed, 28 Aug 2024 21:25:43 -0400 Subject: [PATCH 13/17] Missed one! --- html/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/html/doc.go b/html/doc.go index daf78f8c5..885c4c593 100644 --- a/html/doc.go +++ b/html/doc.go @@ -78,7 +78,7 @@ example, to process each anchor node in depth-first order: if err != nil { // ... } - for n := range doc.All() { + for n := range doc.Descendants() { if n.Type == html.ElementNode && n.Data == "a" { // Do something with n... } From 3cbdcf16491821789728ba301f5bf81c988a9153 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Wed, 28 Aug 2024 21:27:30 -0400 Subject: [PATCH 14/17] Bring back the mutant warning --- html/iter.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/html/iter.go b/html/iter.go index 59b96649e..ae65bc25e 100644 --- a/html/iter.go +++ b/html/iter.go @@ -30,6 +30,8 @@ func (n *Node) Ancestors() iter.Seq[*Node] { // Example: // // for child := range n.Children() { ... } +// +// Mutating a Node or its children while iterating may have unexpected results. func (n *Node) Children() iter.Seq[*Node] { _ = n.FirstChild // eager nil check @@ -46,6 +48,8 @@ func (n *Node) Children() iter.Seq[*Node] { // Example: // // for desc := range n.Descendants() { ... } +// +// Mutating a Node or its descendants while iterating may have unexpected results. func (n *Node) Descendants() iter.Seq[*Node] { _ = n.FirstChild // eager nil check From ac0b9cf6560ff9fb9762e0f56e062b7fa504ae93 Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Thu, 29 Aug 2024 10:54:50 -0400 Subject: [PATCH 15/17] Simpler test --- html/iter_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/html/iter_test.go b/html/iter_test.go index f3b019518..ad19c6845 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -16,21 +16,21 @@ func TestNode_Children(t *testing.T) { in string want string }{ - {"", ""}, - {"", "b"}, - {"b", "b"}, - {"", "b"}, - {"bd", "b c d"}, - {"be", "b c e"}, - {"dfgj", "b g h i"}, + {"", ""}, + {"", "a"}, + {"a", "a"}, + {"", "a b"}, + {"ac", "a b c"}, + {"ad", "a b d"}, + {"cefi", "a f g h"}, } for _, test := range tests { doc, err := Parse(strings.NewReader(test.in)) if err != nil { t.Fatal(err) } - // Drill to - n := doc.FirstChild.FirstChild.NextSibling.FirstChild + // Drill to + n := doc.FirstChild.FirstChild.NextSibling var results []string for c := range n.Children() { results = append(results, c.Data) From bb50757b28e40ee7557e061af8e00c5c5a9b207d Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Mon, 28 Oct 2024 16:13:59 -0400 Subject: [PATCH 16/17] Rename Children to ChildNodes --- html/iter.go | 8 ++++---- html/iter_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/html/iter.go b/html/iter.go index ae65bc25e..c9878e004 100644 --- a/html/iter.go +++ b/html/iter.go @@ -24,15 +24,15 @@ func (n *Node) Ancestors() iter.Seq[*Node] { } } -// Children returns an iterator over the immediate children of n, +// ChildNodes returns an iterator over the immediate children of n, // starting with n.FirstChild. // // Example: // -// for child := range n.Children() { ... } +// for child := range n.ChildNodes() { ... } // // Mutating a Node or its children while iterating may have unexpected results. -func (n *Node) Children() iter.Seq[*Node] { +func (n *Node) ChildNodes() iter.Seq[*Node] { _ = n.FirstChild // eager nil check return func(yield func(*Node) bool) { @@ -59,7 +59,7 @@ func (n *Node) Descendants() iter.Seq[*Node] { } func (n *Node) descendants(yield func(*Node) bool) bool { - for c := range n.Children() { + for c := range n.ChildNodes() { if !yield(c) || !c.descendants(yield) { return false } diff --git a/html/iter_test.go b/html/iter_test.go index ad19c6845..cc4c7b279 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -11,7 +11,7 @@ import ( "testing" ) -func TestNode_Children(t *testing.T) { +func TestNode_ChildNodes(t *testing.T) { tests := []struct { in string want string @@ -32,11 +32,11 @@ func TestNode_Children(t *testing.T) { // Drill to n := doc.FirstChild.FirstChild.NextSibling var results []string - for c := range n.Children() { + for c := range n.ChildNodes() { results = append(results, c.Data) } if got := strings.Join(results, " "); got != test.want { - t.Errorf("unexpected children yielded by Children; want: %q got: %q", test.want, got) + t.Errorf("unexpected children yielded by ChildNodes; want: %q got: %q", test.want, got) } } } From d99de580ab8c30bb04c9fee401d6bfc5dd55d1ec Mon Sep 17 00:00:00 2001 From: Carlana Johnson Date: Mon, 28 Oct 2024 21:23:11 -0400 Subject: [PATCH 17/17] Feedback --- html/iter.go | 14 +------------- html/iter_test.go | 7 +++---- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/html/iter.go b/html/iter.go index c9878e004..54be8fd30 100644 --- a/html/iter.go +++ b/html/iter.go @@ -10,10 +10,6 @@ import "iter" // Ancestors returns an iterator over the ancestors of n, starting with n.Parent. // -// Example: -// -// for ancestor := range n.Ancestors() { ... } -// // Mutating a Node or its parents while iterating may have unexpected results. func (n *Node) Ancestors() iter.Seq[*Node] { _ = n.Parent // eager nil check @@ -27,10 +23,6 @@ func (n *Node) Ancestors() iter.Seq[*Node] { // ChildNodes returns an iterator over the immediate children of n, // starting with n.FirstChild. // -// Example: -// -// for child := range n.ChildNodes() { ... } -// // Mutating a Node or its children while iterating may have unexpected results. func (n *Node) ChildNodes() iter.Seq[*Node] { _ = n.FirstChild // eager nil check @@ -45,16 +37,12 @@ func (n *Node) ChildNodes() iter.Seq[*Node] { // Descendants returns an iterator over all nodes recursively beneath // n, excluding n itself. Nodes are visited in depth-first preorder. // -// Example: -// -// for desc := range n.Descendants() { ... } -// // Mutating a Node or its descendants while iterating may have unexpected results. func (n *Node) Descendants() iter.Seq[*Node] { _ = n.FirstChild // eager nil check return func(yield func(*Node) bool) { - _ = n.descendants(yield) + n.descendants(yield) } } diff --git a/html/iter_test.go b/html/iter_test.go index cc4c7b279..cca7f82f5 100644 --- a/html/iter_test.go +++ b/html/iter_test.go @@ -36,7 +36,7 @@ func TestNode_ChildNodes(t *testing.T) { results = append(results, c.Data) } if got := strings.Join(results, " "); got != test.want { - t.Errorf("unexpected children yielded by ChildNodes; want: %q got: %q", test.want, got) + t.Errorf("ChildNodes = %q, want %q", got, test.want) } } } @@ -67,8 +67,7 @@ func TestNode_Descendants(t *testing.T) { results = append(results, c.Data) } if got := strings.Join(results, " "); got != test.want { - t.Errorf("unexpected children yielded by Descendants; want: %q got: %q", - test.want, got) + t.Errorf("Descendants = %q; want: %q", got, test.want) } } } @@ -81,7 +80,7 @@ func TestNode_Ancestors(t *testing.T) { nParents++ } if nParents != size { - t.Errorf("unexpected number of Ancestors; want %d got: %d", size, nParents) + t.Errorf("number of Ancestors = %d; want: %d", nParents, size) } } }