-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GO-5364 export markdown in memory #2262
GO-5364 export markdown in memory #2262
Conversation
|
single object export/deeplink linking for md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements an internal export API to export a single object into a Markdown byte slice for the API. Key changes include:
- Adding new export entries to the documentation and proto definitions.
- Creating a new middleware handler (ObjectExport) and its supporting ExportSingleInMemory method.
- Introducing an InMemoryWriter and deepLinkNamer to support in-memory and deep-linked exports.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
docs/proto.md | Added new export-related entries and hyperlinks for RPC calls. |
pkg/lib/mill/image_resize.go | Introduced IsImageExt to support image extension checks. |
core/export.go | Added ObjectExport middleware and ExportSingleInMemory for in-memory exports. |
pb/protos/commands.proto | Added new Export Request/Response messages and updated comments in the proto. |
core/block/export/export.go | Implemented exportObject for synchronous single-object export using InMemoryWriter. |
core/block/import/markdown/anymark/anyblocks.go | Replaced hardcoded image extension list with mill.IsImageExt. |
pb/protos/service/service.proto | Added ObjectExport RPC definition. |
core/block/export/writer.go | Updated writer interface and added InMemoryWriter and deepLinkNamer for link exports. |
Comments suppressed due to low confidence (2)
pb/protos/commands.proto:2879
- [nitpick] Consider updating the comment to 'ID of document for export' to match the singular field name 'objectId' and improve clarity.
// ids of documents for export, when empty - will export all available docs
core/block/export/writer.go:203
- Ensure that the 'ext' parameter is consistently formatted (with or without a leading period) when used in condition checks. For example, if ext is not normalized prior to comparison, deepLinkNamer.Get may produce unexpected results.
if ext == ".md" {
for _, v := range inMemoryWriter.data { | ||
if e.format == model.Export_Protobuf { | ||
return base64.StdEncoding.EncodeToString(v), nil | ||
} | ||
return string(v), nil | ||
} | ||
|
||
return "", fmt.Errorf("failed to find data in writer") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loop in exportObject returns on the first iteration; if multiple file entries are ever present, only the first will be processed. Consider asserting that only one file entry exists or explicitly handling multiple entries.
for _, v := range inMemoryWriter.data { | |
if e.format == model.Export_Protobuf { | |
return base64.StdEncoding.EncodeToString(v), nil | |
} | |
return string(v), nil | |
} | |
return "", fmt.Errorf("failed to find data in writer") | |
var result string | |
for _, v := range inMemoryWriter.data { | |
if e.format == model.Export_Protobuf { | |
result += base64.StdEncoding.EncodeToString(v) | |
} else { | |
result += string(v) | |
} | |
} | |
if result == "" { | |
return "", fmt.Errorf("failed to find data in writer") | |
} | |
return result, nil |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Internal export API to export a single object into markdown byte slice. To be used in API