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.

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

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

merged 3 commits into from Oct 26, 2021

Conversation

anthonygedeon
Copy link
Contributor

Fixes #377 issue

@anthonygedeon anthonygedeon changed the title Fixes PuerkitoBio#377, renders the html that can be manipulated with writer Fixes #377, renders the html that can be manipulated with writer Oct 25, 2021
Copy link
Member

@mna mna left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, it looks great! Left just a couple of minor things, if you have the time to address them, great, otherwise I can take care of those later this week.

utilities.go Outdated
@@ -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

@@ -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.

utilities.go Outdated
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!

@mna mna merged commit 4110e17 into PuerkitoBio:master Oct 26, 2021
@mna
Copy link
Member

mna commented Oct 26, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is it possible to save html that was manipulated using goquery?
2 participants