From c34a1a033eb83440d20db24690465b5dc58b013e Mon Sep 17 00:00:00 2001 From: Joakim Date: Fri, 24 Oct 2025 20:53:49 +0200 Subject: [PATCH] Manual code review by an actual human. --- internal/engine/engine.go | 19 ++++++++--------- internal/engine/file.go | 2 +- internal/engine/utils.go | 43 ++++++++------------------------------- 3 files changed, 18 insertions(+), 46 deletions(-) diff --git a/internal/engine/engine.go b/internal/engine/engine.go index d35fecb..0a4c1d9 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -220,13 +220,12 @@ func (e *ContentEngine) hasInsertrAddClass(node *html.Node) bool { } // addContentAttributes adds data-content-id attribute only -// HTML-first approach: no content-type attribute needed func (e *ContentEngine) addContentAttributes(node *html.Node, contentID string) { // Add data-content-id attribute e.setAttribute(node, "data-content-id", contentID) } -// setAttribute sets an attribute on an HTML node +// setAttribute sets an attribute on a HTML node func (e *ContentEngine) setAttribute(node *html.Node, key, value string) { // Remove existing attribute if it exists for i, attr := range node.Attr { @@ -262,10 +261,8 @@ func (e *ContentEngine) addClass(node *html.Node, className string) { } // Check if class already exists - for _, class := range classes { - if class == className { - return // Class already exists - } + if slices.Contains(classes, className) { + return } // Add new class @@ -314,7 +311,7 @@ func (e *ContentEngine) removeClass(node *html.Node, className string) { // Update or remove class attribute if len(newClasses) == 0 { // Remove class attribute entirely if no classes remain - node.Attr = append(node.Attr[:classIndex], node.Attr[classIndex+1:]...) + node.Attr = slices.Delete(node.Attr, classIndex, classIndex+1) } else { // Update class attribute with remaining classes node.Attr[classIndex].Val = strings.Join(newClasses, " ") @@ -338,6 +335,7 @@ func (e *ContentEngine) injectContent(elements []ProcessedElement, siteID string elem.Content = contentItem.HTMLContent // Update injector siteID for this operation + // HACK: I do not like this. Injector refactor? e.injector.siteID = siteID e.injector.injectHTMLContent(elem.Node, contentItem.HTMLContent) } @@ -360,6 +358,7 @@ func (e *ContentEngine) extractHTMLContent(node *html.Node) string { } // extractOriginalTemplate extracts the outer HTML of the element (including the element itself) +// HACK: Rename func (e *ContentEngine) extractOriginalTemplate(node *html.Node) string { var buf strings.Builder if err := html.Render(&buf, node); err != nil { @@ -418,10 +417,8 @@ func (e *ContentEngine) hasClass(n *html.Node, className string) bool { for _, attr := range n.Attr { if attr.Key == "class" { classes := strings.Fields(attr.Val) - for _, class := range classes { - if class == className { - return true - } + if slices.Contains(classes, className) { + return true } } } diff --git a/internal/engine/file.go b/internal/engine/file.go index ce53f59..251e18f 100644 --- a/internal/engine/file.go +++ b/internal/engine/file.go @@ -62,7 +62,7 @@ func (e *ContentEngine) ProcessDirectory(inputDir, outputDir, siteID string, mod }) } -// writeHTMLDocument writes an HTML document to a file +// writeHTMLDocument writes a HTML document to a file func writeHTMLDocument(outputPath string, doc *html.Node) error { // Create output directory if err := os.MkdirAll(filepath.Dir(outputPath), 0755); err != nil { diff --git a/internal/engine/utils.go b/internal/engine/utils.go index 9acba3e..e94a6b6 100644 --- a/internal/engine/utils.go +++ b/internal/engine/utils.go @@ -7,9 +7,8 @@ import ( "slices" ) -// GetClasses extracts CSS classes from an HTML node func GetClasses(node *html.Node) []string { - classAttr := getAttribute(node, "class") + classAttr := GetAttribute(node, "class") if classAttr == "" { return []string{} } @@ -23,8 +22,8 @@ func ContainsClass(classes []string, target string) bool { return slices.Contains(classes, target) } -// getAttribute gets an attribute value from an HTML node -func getAttribute(node *html.Node, key string) string { +// GetAttribute gets an attribute value from an HTML node +func GetAttribute(node *html.Node, key string) string { for _, attr := range node.Attr { if attr.Key == key { return attr.Val @@ -76,9 +75,9 @@ var blockingElements = map[string]bool{ "dl": true, } -// hasEditableContent checks if a node contains content that can be safely edited +// HasEditableContent checks if a node contains content that can be safely edited // This includes text and safe inline formatting elements -func hasEditableContent(node *html.Node) bool { +func HasEditableContent(node *html.Node) bool { if node.Type != html.ElementNode { return false } @@ -129,16 +128,15 @@ func isContainer(node *html.Node) bool { "main": true, "aside": true, "nav": true, - "ul": true, // Phase 3: Lists are containers + "ul": true, "ol": true, } return containerTags[node.Data] } -// findViableChildren finds all descendant elements that should get .insertr class -// Phase 3: Recursive traversal with block/inline classification and boundary respect -func findViableChildren(node *html.Node) []*html.Node { +// FindViableChildren finds all descendant elements that should get .insertr class +func FindViableChildren(node *html.Node) []*html.Node { var viable []*html.Node traverseForViableElements(node, &viable) return viable @@ -266,12 +264,7 @@ func isDeferredElement(node *html.Node) bool { // hasInsertrClass checks if node has class="insertr" func hasInsertrClass(node *html.Node) bool { classes := GetClasses(node) - for _, class := range classes { - if class == "insertr" { - return true - } - } - return false + return slices.Contains(classes, "insertr") } // isSelfClosing checks if an element is typically self-closing @@ -331,23 +324,6 @@ func findElementWithContent(node *html.Node, targetTag, targetContent string) *h return nil } -// GetAttribute gets an attribute value from an HTML node (exported version) -func GetAttribute(node *html.Node, key string) string { - return getAttribute(node, key) -} - -// HasEditableContent checks if a node has editable content (exported version) -func HasEditableContent(node *html.Node) bool { - return hasEditableContent(node) -} - -// FindViableChildren finds viable children for editing (exported version) -func FindViableChildren(node *html.Node) []*html.Node { - return findViableChildren(node) -} - -// Text extraction utility functions - // ExtractTextContent extracts all text content from an HTML node recursively func ExtractTextContent(node *html.Node) string { var text strings.Builder @@ -355,7 +331,6 @@ func ExtractTextContent(node *html.Node) string { return strings.TrimSpace(text.String()) } -// extractTextRecursiveUnified is the internal unified implementation func extractTextRecursiveUnified(node *html.Node, text *strings.Builder) { if node.Type == html.TextNode { text.WriteString(node.Data)