From 89b1e48e693e295044c241f370859662ac6bc28f Mon Sep 17 00:00:00 2001 From: Asaf Gartner Date: Sat, 6 Aug 2022 00:41:14 +0300 Subject: [PATCH] Code review --- public/style.css | 18 ------ src/embed/embed.go | 59 +++++++++---------- src/rawdata/scss/_core.scss | 24 -------- .../src/include/showcase_templates.html | 2 +- src/templates/src/include/snippet_edit.html | 2 +- src/templates/src/include/timeline_item.html | 2 +- 6 files changed, 30 insertions(+), 77 deletions(-) diff --git a/public/style.css b/public/style.css index ca3b1a4..f91620b 100644 --- a/public/style.css +++ b/public/style.css @@ -7464,24 +7464,6 @@ article code { .h1-5 { height: 1.5rem; } -.gap0 { - gap: 0; } - -.gap1 { - gap: 0.25rem; } - -.gap2 { - gap: 0.5rem; } - -.gap3 { - gap: 1rem; } - -.gap4 { - gap: 2rem; } - -.gap5 { - gap: 4rem; } - .pre-line { white-space: pre-line; } diff --git a/src/embed/embed.go b/src/embed/embed.go index 3912e90..8eadcd7 100644 --- a/src/embed/embed.go +++ b/src/embed/embed.go @@ -11,7 +11,6 @@ import ( "strings" "time" - "git.handmade.network/hmn/hmn/src/logging" "git.handmade.network/hmn/hmn/src/utils" ) @@ -37,6 +36,9 @@ func IsUrlEmbeddable(u string) bool { func GetEmbeddableFromUrls(ctx context.Context, urls []string, maxSize int, httpTimeout time.Duration, httpMaxAttempts int) (*Embeddable, error) { embedError := NoEmbedFound + client := &http.Client{ + Timeout: httpTimeout, + } for _, urlStr := range urls { u, err := url.Parse(urlStr) if err != nil { @@ -60,7 +62,7 @@ func GetEmbeddableFromUrls(ctx context.Context, urls []string, maxSize int, http if httpMaxAttempts > 0 { httpMaxAttempts -= 1 - embed, err := FetchEmbed(ctx, urlStr, httpTimeout, maxSize) + embed, err := FetchEmbed(ctx, urlStr, client, maxSize) if err != nil { embedError = err continue @@ -77,34 +79,28 @@ func GetEmbeddableFromUrls(ctx context.Context, urls []string, maxSize int, http // If the url points to a file, downloads and returns the file. // If the url points to an html page, parses opengraph and tries to fetch an image/video/audio file according to that. // maxSize only limits the actual filesize. In the case of html we always fetch up to 100kb even if maxSize is smaller. -func FetchEmbed(ctx context.Context, urlStr string, timeout time.Duration, maxSize int) (*Embed, error) { - logging.ExtractLogger(ctx).Debug().Msg("Fetching embed") - client := &http.Client{ - Timeout: timeout, - } +func FetchEmbed(ctx context.Context, urlStr string, httpClient *http.Client, maxSize int) (*Embed, error) { req, err := http.NewRequestWithContext(ctx, "GET", urlStr, nil) if err != nil { return nil, err } - res, err := client.Do(req) + res, err := httpClient.Do(req) if err != nil { return nil, err } + defer res.Body.Close() if res.StatusCode < 200 || res.StatusCode > 299 { return nil, NoEmbedFound } contentType := res.Header.Get("Content-Type") - logging.ExtractLogger(ctx).Debug().Str("type", contentType).Msg("Got first result") if strings.HasPrefix(contentType, "text/html") || strings.HasPrefix(contentType, "application/html") { var buffer bytes.Buffer _, err := io.CopyN(&buffer, res.Body, 100*1024) // NOTE(asaf): If the opengraph stuff isn't in the first 100kb, we don't care. - res.Body.Close() if err != nil && !errors.Is(err, io.EOF) { return nil, err } partialHtml := buffer.Bytes() urlStr = ExtractEmbedFromOpenGraph(partialHtml) - logging.ExtractLogger(ctx).Debug().Str("url", urlStr).Msg("Got ograph") if urlStr == "" { return nil, NoEmbedFound } @@ -113,10 +109,11 @@ func FetchEmbed(ctx context.Context, urlStr string, timeout time.Duration, maxSi if err != nil { return nil, err } - res, err = client.Do(req) + res, err = httpClient.Do(req) if err != nil { return nil, err } + defer res.Body.Close() if res.StatusCode < 200 || res.StatusCode > 299 { return nil, NoEmbedFound } @@ -125,7 +122,6 @@ func FetchEmbed(ctx context.Context, urlStr string, timeout time.Duration, maxSi var buffer bytes.Buffer n, err := io.CopyN(&buffer, res.Body, int64(maxSize+1)) - res.Body.Close() if err != nil && !errors.Is(err, io.EOF) { return nil, err } @@ -171,28 +167,27 @@ func ExtractEmbedFromOpenGraph(partialHtml []byte) string { keyIdx := metaAttrRegex.SubexpIndex("key") valueIdx := metaAttrRegex.SubexpIndex("value") html := string(partialHtml) - matches := metaRegex.FindAllStringSubmatch(html, -1) - for _, m := range matches { - if len(m) > 1 { - content := "" - prop := "" - attrs := metaAttrRegex.FindAllStringSubmatch(m[1], -1) - for _, attr := range attrs { - key := attr[keyIdx] - value := attr[valueIdx] - if key == "name" || key == "property" { - for _, ogKey := range OGKeys { - if value == ogKey { - prop = value - } + metaTags := metaRegex.FindAllStringSubmatch(html, -1) + for _, m := range metaTags { + content := "" + relevantProp := false + attrs := metaAttrRegex.FindAllStringSubmatch(m[1], -1) + for _, attr := range attrs { + key := attr[keyIdx] + value := attr[valueIdx] + if key == "name" || key == "property" { + for _, ogKey := range OGKeys { + if value == ogKey { + relevantProp = true + break } - } else if key == "content" { - content = value } + } else if key == "content" { + content = value } - if content != "" && prop != "" { - return content - } + } + if content != "" && relevantProp { + return content } } return "" diff --git a/src/rawdata/scss/_core.scss b/src/rawdata/scss/_core.scss index 2392009..f449527 100644 --- a/src/rawdata/scss/_core.scss +++ b/src/rawdata/scss/_core.scss @@ -337,30 +337,6 @@ article code { height: 1.5rem; } -.gap0 { - gap: $spacing-none; -} - -.gap1 { - gap: $spacing-extra-small; -} - -.gap2 { - gap: $spacing-small; -} - -.gap3 { - gap: $spacing-medium; -} - -.gap4 { - gap: $spacing-large; -} - -.gap5 { - gap: $spacing-extra-large; -} - .pre-line { white-space: pre-line; } diff --git a/src/templates/src/include/showcase_templates.html b/src/templates/src/include/showcase_templates.html index 6af8ec2..2730fea 100644 --- a/src/templates/src/include/showcase_templates.html +++ b/src/templates/src/include/showcase_templates.html @@ -30,7 +30,7 @@
Unknown description
-
+
View original message on Discord
diff --git a/src/templates/src/include/snippet_edit.html b/src/templates/src/include/snippet_edit.html index 8c17398..d4688b5 100644 --- a/src/templates/src/include/snippet_edit.html +++ b/src/templates/src/include/snippet_edit.html @@ -28,7 +28,7 @@ -
+
diff --git a/src/templates/src/include/timeline_item.html b/src/templates/src/include/timeline_item.html index cb98ab4..a463e42 100644 --- a/src/templates/src/include/timeline_item.html +++ b/src/templates/src/include/timeline_item.html @@ -69,7 +69,7 @@ {{ end }} {{ with .Projects }} -
+
{{ range $i, $proj := . }}