Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions metal/cli/seo/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ func (g *Generator) GeneratePosts() error {
for _, post := range posts {
cli.Cyanln(fmt.Sprintf("Building SEO for post: %s", post.Slug))
response := payload.GetPostsResponse(post)
cli.Grayln(fmt.Sprintf("Post slug: %s", response.Slug))
cli.Grayln(fmt.Sprintf("Post title: %s", response.Title))
body := []template.HTML{sections.Post(&response)}

data, buildErr := g.BuildForPost(response, body)
Expand Down Expand Up @@ -457,18 +459,42 @@ func (g *Generator) TitleFor(pageName string) string {
return fmt.Sprintf("%s · %s", pageName, g.Page.SiteName)
}

func truncateForLog(value string) string {
const maxRunes = 80

cleaned := strings.TrimSpace(value)
if cleaned == "" {
return "(empty)"
}

if utf8.RuneCountInString(cleaned) <= maxRunes {
return cleaned
}

runes := []rune(cleaned)
return string(runes[:maxRunes-3]) + "..."
}
Comment on lines +462 to +476

Choose a reason for hiding this comment

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

medium

The truncateForLog function could panic if maxRunes is set to a value less than 3. While it's a constant set to 80 for now, it's good practice to make utility functions robust against such changes. I'd suggest adding a check to handle small values of maxRunes gracefully.

func truncateForLog(value string) string {
	const maxRunes = 80

	cleaned := strings.TrimSpace(value)
	if cleaned == "" {
		return "(empty)"
	}

	if utf8.RuneCountInString(cleaned) <= maxRunes {
		return cleaned
	}

	runes := []rune(cleaned)
	if maxRunes < 3 {
		return string(runes[:maxRunes])
	}
	return string(runes[:maxRunes-3]) + "..."
}


func (g *Generator) BuildForPost(post payload.PostResponse, body []template.HTML) (TemplateData, error) {
path := g.CanonicalPostPath(post.Slug)
imageAlt := g.SanitizeAltText(post.Title, g.Page.SiteName)
description := g.SanitizeMetaDescription(post.Excerpt, Description)
image := g.PreferredImageURL(post.CoverImageURL, g.Page.AboutPhotoUrl)
imageType := "image/png"

cli.Grayln(fmt.Sprintf("Preparing post metadata"))
cli.Grayln(fmt.Sprintf(" Canonical path: %s", path))
cli.Grayln(fmt.Sprintf(" Sanitised alt text: %s", imageAlt))
cli.Grayln(fmt.Sprintf(" Description preview: %s", truncateForLog(description)))
cli.Grayln(fmt.Sprintf(" Preferred image candidate: %s", image))

if prepared, err := g.preparePostImage(post); err == nil && prepared.URL != "" {
cli.Grayln(fmt.Sprintf(" Post image prepared at: %s (%s)", prepared.URL, prepared.Mime))
image = prepared.URL
imageType = prepared.Mime
} else if err != nil {
cli.Errorln(fmt.Sprintf("failed to prepare post image for %s: %v", post.Slug, err))
cli.Grayln(fmt.Sprintf(" Falling back to preferred image URL: %s", image))
}

return g.buildForPage(post.Title, path, body, func(data *TemplateData) {
Expand Down
13 changes: 13 additions & 0 deletions metal/cli/seo/pictures.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@ const (
func (g *Generator) preparePostImage(post payload.PostResponse) (preparedImage, error) {
source := strings.TrimSpace(post.CoverImageURL)
if source == "" {
cli.Grayln(fmt.Sprintf("Skipping image preparation for %s: no cover image", post.Slug))
return preparedImage{}, errors.New("post has no cover image url")
}

cli.Grayln(fmt.Sprintf("Preparing post image for %s from %s", post.Slug, source))

spaImagesDir, err := g.spaImagesDir()
if err != nil {
cli.Errorln(fmt.Sprintf("Could not determine SPA images directory: %v", err))

Choose a reason for hiding this comment

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

medium

This error log is redundant. The error is returned and logged by the caller in BuildForPost, which is the correct place to handle and log it. Logging in both places creates unnecessary noise. Please remove this line.

This applies to the other new error logs in this function on lines 65, 71, and 77 as well.

return preparedImage{}, err
}

Expand All @@ -48,22 +52,29 @@ func (g *Generator) preparePostImage(post payload.PostResponse) (preparedImage,
return preparedImage{}, err
}

bounds := img.Bounds()
cli.Grayln(fmt.Sprintf("Fetched image %s (%s) with bounds %dx%d", source, format, bounds.Dx(), bounds.Dy()))

resized := pkgimages.Resize(img, seoImageWidth, seoImageHeight)
cli.Grayln(fmt.Sprintf("Resized image to %dx%d", seoImageWidth, seoImageHeight))

ext := pkgimages.DetermineExtension(source, format)
fileName := pkgimages.BuildFileName(post.Slug, ext, "post-image")

if err := os.MkdirAll(spaImagesDir, 0o755); err != nil {
cli.Errorln(fmt.Sprintf("Unable to create SPA images directory %s: %v", spaImagesDir, err))
return preparedImage{}, fmt.Errorf("create destination dir: %w", err)
}

destPath := filepath.Join(spaImagesDir, fileName)
if err := pkgimages.Save(destPath, resized, ext, pkgimages.DefaultJPEGQuality); err != nil {
cli.Errorln(fmt.Sprintf("Unable to save resized image to %s: %v", destPath, err))
return preparedImage{}, fmt.Errorf("write resized image: %w", err)
}

relativeDir, err := filepath.Rel(g.Page.OutputDir, spaImagesDir)
if err != nil {
cli.Errorln(fmt.Sprintf("Unable to determine relative path for %s: %v", spaImagesDir, err))
return preparedImage{}, fmt.Errorf("determine relative image path: %w", err)
}

Expand All @@ -73,6 +84,8 @@ func (g *Generator) preparePostImage(post payload.PostResponse) (preparedImage,
relative := path.Join(relativeDir, fileName)
relative = pkgimages.NormalizeRelativeURL(relative)

cli.Grayln(fmt.Sprintf("Post image relative path: %s", relative))

return preparedImage{
URL: g.siteURLFor(relative),
Mime: pkgimages.MIMEFromExtension(ext),
Expand Down
Loading