Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #377, renders the html that can be manipulated with writer #399

Merged
merged 3 commits into from Oct 26, 2021
Merged

Fixes #377, renders the html that can be manipulated with writer #399

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98eb88c
Fixes PuerkitoBio#377, renders the html that can be manipulated with …
anthonygedeon Oct 25, 2021
11e1233
fix(test): remove Render testcase since OuterHtml handles Render inte…
anthonygedeon Oct 25, 2021
d35d4ce
fix(doc): update docs for accurate representation of function
anthonygedeon Oct 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions utilities.go
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@ package goquery

import (
"bytes"
"io"

"golang.org/x/net/html"
)
Expand Down Expand Up @@ -57,6 +58,19 @@ func nodeName(node *html.Node) string {
}
}

// Render renders the html of the first element from selector and writes it to the writer.
// It behaves similar to OuterHtml but takes io.Writer as input.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// It behaves similar to OuterHtml but takes io.Writer as input.
// It behaves the same as OuterHtml but writes to w instead of returning the string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Done

func Render(w io.Writer, s *Selection) error {
if s.Length() == 0 {
return nil
}
n := s.Get(0)
if err := html.Render(w, n); err != nil {
return err
}
return nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh one more thing, this (lines 68-71) could be simplified to just return html.Render(w, n).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

}

// OuterHtml returns the outer HTML rendering of the first item in
// the selection - that is, the HTML including the first element's
// tag and attributes.
Expand All @@ -66,12 +80,7 @@ func nodeName(node *html.Node) string {
// a property provided by the DOM).
func OuterHtml(s *Selection) (string, error) {
var buf bytes.Buffer

if s.Length() == 0 {
return "", nil
}
n := s.Get(0)
if err := html.Render(&buf, n); err != nil {
if err := Render(&buf, s); err != nil {
return "", err
}
return buf.String(), nil
Expand Down
89 changes: 51 additions & 38 deletions utilities_test.go
View file
Open in desktop
Expand Up @@ -24,6 +24,49 @@ var allNodes = `<!doctype html>
</body>
</html>`

func assertMatchNodes(t testing.TB, doc *Document) {
t.Helper()

n0 := doc.Nodes[0]
nDT := n0.FirstChild
sMeta := doc.Find("meta")
sP := doc.Find("p")
nP := sP.Get(0)
nComment := nP.FirstChild
nText := nComment.NextSibling
sHeaders := doc.Find(".header")

cases := []struct {
node *html.Node
sel *Selection
want string
}{
{nDT, nil, "<!DOCTYPE html>"}, // render makes DOCTYPE all caps
{nil, sMeta, `<meta a="b"/>`}, // and auto-closes the meta
{nil, sP, `<p><!-- this is a comment -->
This is some text.
</p>`},
{nComment, nil, "<!-- this is a comment -->"},
{nText, nil, `
This is some text.
`},
{nil, sHeaders, `<h1 class="header"></h1>`},
}
for i, c := range cases {
if c.sel == nil {
c.sel = newSingleSelection(c.node, doc)
}
got, err := OuterHtml(c.sel)
if err != nil {
t.Fatal(err)
}

if got != c.want {
t.Errorf("%d: want %q, got %q", i, c.want, got)
}
}
}

func TestNodeName(t *testing.T) {
doc, err := NewDocumentFromReader(strings.NewReader(allNodes))
if err != nil {
Expand Down Expand Up @@ -81,48 +124,18 @@ func TestNodeNameMultiSel(t *testing.T) {
}
}

func TestOuterHtml(t *testing.T) {
func TestRender(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, given that this is exactly the same as TestOuterHtml and that OuterHtml calls Render anyway, it's fine to keep just TestOuterHtml.

doc, err := NewDocumentFromReader(strings.NewReader(allNodes))
if err != nil {
t.Fatal(err)
}
assertMatchNodes(t, doc)
}

n0 := doc.Nodes[0]
nDT := n0.FirstChild
sMeta := doc.Find("meta")
sP := doc.Find("p")
nP := sP.Get(0)
nComment := nP.FirstChild
nText := nComment.NextSibling
sHeaders := doc.Find(".header")

cases := []struct {
node *html.Node
sel *Selection
want string
}{
{nDT, nil, "<!DOCTYPE html>"}, // render makes DOCTYPE all caps
{nil, sMeta, `<meta a="b"/>`}, // and auto-closes the meta
{nil, sP, `<p><!-- this is a comment -->
This is some text.
</p>`},
{nComment, nil, "<!-- this is a comment -->"},
{nText, nil, `
This is some text.
`},
{nil, sHeaders, `<h1 class="header"></h1>`},
}
for i, c := range cases {
if c.sel == nil {
c.sel = newSingleSelection(c.node, doc)
}
got, err := OuterHtml(c.sel)
if err != nil {
t.Fatal(err)
}

if got != c.want {
t.Errorf("%d: want %q, got %q", i, c.want, got)
}
func TestOuterHtml(t *testing.T) {
doc, err := NewDocumentFromReader(strings.NewReader(allNodes))
if err != nil {
t.Fatal(err)
}
assertMatchNodes(t, doc)
}