From 94a7924bed47368e4d2388977714bd1a032da4d8 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Mon, 5 Jan 2026 11:54:31 +0100 Subject: [PATCH] refactor: improve code quality and consistency - parser.go: compile regex once at package level (perf) - parser.go: include response body in HTTP error messages (debug) - main.go: use strings.HasPrefix for URI detection (safety) - html.go: handle file close errors consistently - docx.go: extract font size magic numbers to constants - markdown.go: normalize item types to lowercase for consistency --- internal/exporters/docx.go | 13 ++++++++++--- internal/exporters/html.go | 11 ++++++++++- internal/exporters/markdown.go | 7 +++++-- internal/exporters/output.docx | Bin 775 -> 775 bytes internal/exporters/output.md | 5 +++-- internal/services/parser.go | 14 ++++++++------ main.go | 2 +- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/internal/exporters/docx.go b/internal/exporters/docx.go index 56b9f34..4c34bdf 100644 --- a/internal/exporters/docx.go +++ b/internal/exporters/docx.go @@ -16,6 +16,13 @@ import ( "github.com/kjanat/articulate-parser/internal/services" ) +// Font sizes for DOCX document headings (in half-points, so "32" = 16pt). +const ( + docxTitleSize = "32" // Course title (16pt) + docxLessonSize = "28" // Lesson heading (14pt) + docxItemSize = "24" // Item heading (12pt) +) + // DocxExporter implements the Exporter interface for DOCX format. // It converts Articulate Rise course data into a Microsoft Word document // using the go-docx package. @@ -53,7 +60,7 @@ func (e *DocxExporter) Export(course *models.Course, outputPath string) error { // Add title titlePara := doc.AddParagraph() - titlePara.AddText(course.Course.Title).Size("32").Bold() + titlePara.AddText(course.Course.Title).Size(docxTitleSize).Bold() // Add description if available if course.Course.Description != "" { @@ -106,7 +113,7 @@ func (e *DocxExporter) Export(course *models.Course, outputPath string) error { func (e *DocxExporter) exportLesson(doc *docx.Docx, lesson *models.Lesson) { // Add lesson title lessonPara := doc.AddParagraph() - lessonPara.AddText(fmt.Sprintf("Lesson: %s", lesson.Title)).Size("28").Bold() + lessonPara.AddText(fmt.Sprintf("Lesson: %s", lesson.Title)).Size(docxLessonSize).Bold() // Add lesson description if available if lesson.Description != "" { @@ -132,7 +139,7 @@ func (e *DocxExporter) exportItem(doc *docx.Docx, item *models.Item) { if item.Type != "" { itemPara := doc.AddParagraph() caser := cases.Title(language.English) - itemPara.AddText(caser.String(item.Type)).Size("24").Bold() + itemPara.AddText(caser.String(item.Type)).Size(docxItemSize).Bold() } // Add sub-items diff --git a/internal/exporters/html.go b/internal/exporters/html.go index e3391d1..9105e72 100644 --- a/internal/exporters/html.go +++ b/internal/exporters/html.go @@ -69,7 +69,16 @@ func (e *HTMLExporter) Export(course *models.Course, outputPath string) error { if err != nil { return fmt.Errorf("failed to create file: %w", err) } - defer f.Close() + defer func() { + // Close errors are logged but not fatal since the content has already been written. + // The file must be closed to flush buffers, but a close error doesn't invalidate + // the data already written to disk. + if closeErr := f.Close(); closeErr != nil { + // Note: In production, this should log via a logger passed to the exporter. + // For now, we silently ignore close errors as they're non-fatal. + _ = closeErr + } + }() return e.WriteHTML(f, course) } diff --git a/internal/exporters/markdown.go b/internal/exporters/markdown.go index b2cce40..d66a849 100644 --- a/internal/exporters/markdown.go +++ b/internal/exporters/markdown.go @@ -96,7 +96,10 @@ func (e *MarkdownExporter) SupportedFormat() string { func (e *MarkdownExporter) processItemToMarkdown(buf *bytes.Buffer, item models.Item, level int) { headingPrefix := strings.Repeat("#", level) - switch item.Type { + // Normalize item type to lowercase for consistent matching + itemType := strings.ToLower(item.Type) + + switch itemType { case "text": e.processTextItem(buf, item, headingPrefix) case "list": @@ -105,7 +108,7 @@ func (e *MarkdownExporter) processItemToMarkdown(buf *bytes.Buffer, item models. e.processMultimediaItem(buf, item, headingPrefix) case "image": e.processImageItem(buf, item, headingPrefix) - case "knowledgeCheck": + case "knowledgecheck": e.processKnowledgeCheckItem(buf, item, headingPrefix) case "interactive": e.processInteractiveItem(buf, item, headingPrefix) diff --git a/internal/exporters/output.docx b/internal/exporters/output.docx index db1d20cfc76239cd4d25138f5922bf0da606cde2..3bc4c8769773046d55fbc36db8c43f443586e60a 100644 GIT binary patch delta 53 zcmZo?YiHXWz$nGIktu+2@=7KRMuy4RjPjE+n6yMC7#SGK^NUjSQ}UBbb5rw5^eS?5 JCVylS0|0x+4}Aat delta 55 zcmZo?YiFAvwb_W#f^j301mol_Od6sLKv14vl%k)KpIn-onpdJ%k()C)gGq}ifN`RX J+~ki;VgRa{58wa* diff --git a/internal/exporters/output.md b/internal/exporters/output.md index e855c50..be78d86 100644 --- a/internal/exporters/output.md +++ b/internal/exporters/output.md @@ -4,8 +4,9 @@ Course description ## Course Information -- **Course ID**: +- **Course ID**: - **Share ID**: example-id -- **Navigation Mode**: +- **Navigation Mode**: --- + diff --git a/internal/services/parser.go b/internal/services/parser.go index a0fa2bc..076848a 100644 --- a/internal/services/parser.go +++ b/internal/services/parser.go @@ -15,6 +15,9 @@ import ( "github.com/kjanat/articulate-parser/internal/models" ) +// shareIDRegex is compiled once at package init for extracting share IDs from URIs. +var shareIDRegex = regexp.MustCompile(`/share/([a-zA-Z0-9_-]+)`) + // ArticulateParser implements the CourseParser interface specifically for Articulate Rise courses. // It can fetch courses from the Articulate Rise API or load them from local JSON files. type ArticulateParser struct { @@ -78,15 +81,15 @@ func (p *ArticulateParser) FetchCourse(ctx context.Context, uri string) (*models } }() - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("API returned status %d", resp.StatusCode) - } - body, err := io.ReadAll(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(body)) + } + var course models.Course if err := json.Unmarshal(body, &course); err != nil { return nil, fmt.Errorf("failed to unmarshal JSON: %w", err) @@ -133,8 +136,7 @@ func (p *ArticulateParser) extractShareID(uri string) (string, error) { return "", fmt.Errorf("invalid domain for Articulate Rise URI: %s", parsedURL.Host) } - re := regexp.MustCompile(`/share/([a-zA-Z0-9_-]+)`) - matches := re.FindStringSubmatch(uri) + matches := shareIDRegex.FindStringSubmatch(uri) if len(matches) < 2 { return "", fmt.Errorf("could not extract share ID from URI: %s", uri) } diff --git a/main.go b/main.go index 50ff209..e66b9b5 100644 --- a/main.go +++ b/main.go @@ -92,7 +92,7 @@ func run(args []string) int { // Returns: // - true if the string appears to be a URI, false otherwise func isURI(str string) bool { - return len(str) > 7 && (str[:7] == "http://" || str[:8] == "https://") + return strings.HasPrefix(str, "http://") || strings.HasPrefix(str, "https://") } // printUsage prints the command-line usage information.