Image #195
Image #195
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
image/README.md (1)
124-126: Document LoadFromReader.The code exposes
LoadFromReader(format ImageFormat, r io.Reader)but README omits it.Add:
+### LoadFromReader + +- `LoadFromReader(format ImageFormat, r io.Reader) (*Image, error)` + Loads an image from any reader with an explicit format.image/image.go (2)
239-254: WEBP encode unsupported — return clearer error or add support.
encodeTolacks aFormatWEBPcase. Either add encode support (via a third-party lib) or return an error that explicitly says “WEBP encoding not supported”.Apply minimal clarity:
default: - return fmt.Errorf("unable to encode image in format %s", img.Format) + return fmt.Errorf("unable to encode image in format %s (encoding not implemented)", img.Format) }And mirror this limitation in README (see separate comment).
138-146: Optional: offer data-URI variant for ToBase64.Common consumers expect
data:image/<fmt>;base64,<payload>.Sample helper (outside selected range):
func (img *Image) ToDataURI() (string, error) { b64, err := img.ToBase64() if err != nil { return "", err } return fmt.Sprintf("data:image/%s;base64,%s", img.Format, b64), nil }image/EXAMPLES.md (5)
8-13: Alias the utils package to avoid colliding with the stdlibimagepackage.Using
imageas the import name forgithub.aichem.org/kashifkhan0771/utils/imagewill commonly clash withimagefrom the standard library in real apps. Recommend aliasing toimgutil(or similar) across all examples.Apply to each example; e.g., first snippet:
import ( "fmt" "log" - "github.com/kashifkhan0771/utils/image" + imgutil "github.com/kashifkhan0771/utils/image" ) func main() { path := "example.png" - img, err := image.LoadFromFile(path) + img, err := imgutil.LoadFromFile(path)Also applies to: 18-19, 40-41, 46-47, 68-69, 73-74, 105-107, 111-112, 133-135, 139-140, 161-163, 167-168, 193-195, 199-200, 225-227, 231-232, 255-256, 260-261
31-31: Grammar: add article “a”.Change to “Load an image from a URL”.
-### Load an image from URL +### Load an image from a URL
92-95: Avoid brittle, exact byte counts in example output.The lengths vary by encoder/version and remote content. Use non-literal placeholders.
-#### Output -`Byte length: 829 -Base64 length: 1108` +#### Output (illustrative) +`Byte length: <non-zero> +Base64 length: <non-zero>`
106-107: Consider alternatives tonfnt/resize(archived).
github.com/nfnt/resizehas long been archived. Prefergolang.org/x/image/draw(CatmullRom, ApproxBiLinear) or a maintained library likegithub.aichem.org/disintegration/imaging. If you keepnfnt/resize, note its status in README and examples.Would you like me to update the examples to show
x/image/drawequivalents?Also applies to: 134-135, 162-163, 194-195, 226-227
96-123: Add examples forResizeToWidthandResizeToHeight.Tests exercise these APIs but examples omit them. Adding small snippets will close the doc gap and clarify aspect-ratio semantics.
I can draft concise examples that mirror the test expectations (one-dimension resize without aspect-ratio changes). Want me to add them?
image/image_test.go (1)
1-1: Consider external test package to validate public API only.Switching
package image→package image_testavoids internal coupling and the stdlib name conflict entirely (withstdimagealias). Optional but cleaner for consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
go.mod(1 hunks)image/EXAMPLES.md(1 hunks)image/README.md(1 hunks)image/image.go(1 hunks)image/image_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
image/image_test.go (1)
image/image.go (14)
Image(24-32)ImageFormat(34-34)FormatPNG(41-41)FormatJPEG(40-40)FormatJFIF(38-38)FormatJPE(39-39)FormatGIF(42-42)FormatTIF(43-43)FormatTIFF(44-44)FormatTIFF_FX(45-45)FormatWEBP(46-46)FormatBMP(47-47)FormatDIB(48-48)FormatXBMP(49-49)
image/image.go (1)
math/math.go (1)
Min(43-49)
🪛 LanguageTool
image/EXAMPLES.md
[grammar] ~31-~31: There might be a mistake here.
Context: ...=png, size=150x150` ### Load an image from URL ```go package main import ( "fmt...
(QB_NEW_EN)
🔇 Additional comments (1)
image/image_test.go (1)
34-47: No changes required for BMP/DIB/XBMP/TIF/TIFF_FX—encodeTo implements them via golang.org/x/image, and tests correctly expect success.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
image/image.go (3)
64-71: Optional: auto-detect format when file has no extension.Improves UX for extension-less paths by sniffing format instead of failing.
- ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(path)), ".") - if ext == "" { - return nil, fmt.Errorf("unable to determine image format from path (no extension): %s", path) - } - format := ImageFormat(ext) - - return LoadFromReader(format, f) + ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(path)), ".") + if ext == "" { + // Fallback: sniff by decoding + i, formatStr, derr := image.Decode(f) + if derr != nil { + return nil, fmt.Errorf("unable to determine image format: no extension and sniff failed: %w", derr) + } + return newImage(ImageFormat(strings.ToLower(formatStr)), i), nil + } + return LoadFromReader(ImageFormat(ext), f)
225-229: Fix build break: replace undefined min(...) with math.Min.This won’t compile as-is;
minis not defined.- scale := min(ratioW, ratioH) + scale := math.Min(ratioW, ratioH)
91-103: Harden LoadFromURL: parse Content-Type and add decode fallback (with size cap).
mime.ExtensionsByTypemay fail when parameters are present (e.g.,image/jpeg; charset=binary) or when no mapping exists. Addmime.ParseMediaTypeand fall back to decoding (bounded) when extensions are empty.- extensions, err := mime.ExtensionsByType(resp.Header.Get("Content-Type")) - if err != nil { - return nil, err - } - - if len(extensions) == 0 { - return nil, fmt.Errorf("unable to determine image format: no file extensions found for Content-Type '%s'", resp.Header.Get("Content-Type")) - } - - format := ImageFormat(extensions[0][1:]) - - return LoadFromReader(format, resp.Body) + ct := resp.Header.Get("Content-Type") + mediaType, _, _ := mime.ParseMediaType(ct) + exts, _ := mime.ExtensionsByType(mediaType) + if len(exts) > 0 && exts[0] != "" { + format := ImageFormat(strings.TrimPrefix(strings.ToLower(exts[0]), ".")) + return LoadFromReader(format, resp.Body) + } + // Fallback: sniff by decoding a bounded stream + i, formatStr, derr := image.Decode(io.LimitReader(resp.Body, maxSniffBytes)) + if derr != nil { + return nil, fmt.Errorf("unable to determine image format for Content-Type '%s': %w", ct, derr) + } + return newImage(ImageFormat(strings.ToLower(formatStr)), i), nilAdd this near the top-level (outside the diffed range):
const maxSniffBytes = 32 << 20 // 32 MiB
🧹 Nitpick comments (1)
image/image.go (1)
214-230: Confirm API contract: ScaleDown may return the same instance.When no scaling is needed, the method returns the original pointer. If callers expect a new copy in all cases, consider cloning instead or documenting explicitly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.mod(1 hunks)image/README.md(1 hunks)image/image.go(1 hunks)image/image_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- go.mod
- image/README.md
- image/image_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
image/image.go (1)
math/math.go (2)
Max(52-58)Min(43-49)
Signed-off-by: Danilo Šindrić <93711398+danilovict2@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go.mod (1)
7-7: Consider migrating away from nfnt/resize.If you want an actively maintained path with better resamplers, consider
golang.org/x/image/draworgithub.aichem.org/disintegration/imaging. This can be a follow-up; no blocker here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod(1 hunks)
🔇 Additional comments (2)
go.mod (2)
7-8: Direct deps correctly promoted. LGTM.Removing the
// indirectmarkers for nfnt/resize and x/image matches their direct imports in the image package.
9-10: No issues detected. go mod why confirms both x/net and x/text are required, and build, vet, and tests pass successfully.
Description:
Add Image utils.
Checklist:
make test.make lint.If added a new utility function or updated existing function logic:
EXAMPLES.mdREADME.mdIf added a new utility package:
README.mdutility packages tableSummary by CodeRabbit
New Features
Documentation
Tests
Chores