-
Notifications
You must be signed in to change notification settings - Fork 33
Add Markdown export to WAPI #4
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a new exported Zig function Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant WASM as "WASM API\n(zpdf_extract_all_markdown)"
participant DocStore as "Document Store\n(handle lookup)"
participant Doc as "Document\n.extractAllMarkdown"
participant Alloc as "WASM Allocator"
Client->>WASM: call zpdf_extract_all_markdown(handle, out_len_ptr)
WASM->>DocStore: validate handle & lookup document
alt handle invalid or doc missing
DocStore-->>WASM: null
WASM-->>Client: return null (out_len = 0)
else document found
WASM->>Doc: call extractAllMarkdown(Alloc)
Doc->>Alloc: request buffer
Alloc-->>Doc: buffer ptr + length
Doc-->>WASM: buffer ptr + length
alt length == 0
WASM->>Alloc: free buffer
WASM-->>Client: return null (out_len = 0)
else length > 0
WASM-->>Client: return buffer ptr (out_len = length)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/wapi.zig`:
- Around line 125-135: The zpdf_extract_all_markdown function can return a
pointer for empty buffers which breaks parity with
zpdf_extract_all/zpdf_extract_page; after calling
doc.extractAllMarkdown(wasm_allocator) (in the zpdf_extract_all_markdown branch
that handles documents[idx]), check if result.len == 0 and if so set out_len.* =
0 and return null, otherwise set out_len.* = result.len and return result.ptr;
preserve the existing guard for invalid handle and the final return null for
missing documents.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/wapi.zig`:
- Around line 122-142: The comment in zpdf_extract_all_markdown incorrectly
references toOwnedSlice while the function actually uses doc.extractAllMarkdown;
update the comment to describe that extractAllMarkdown returns an allocated
slice and that we treat zero-length results as "no data" by setting out_len to 0
and returning null; then make the C API (capi.zig) implementation consistent
with zpdf_extract_all_markdown by adding the same empty-buffer check around
extractAllMarkdown's result (check result.len == 0, set out_len.* = 0 and return
null) so both WAPI and C API handle empty slices identically.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/capi.zig`:
- Around line 194-200: The zero-length buffer returned by
doc.extractAllMarkdown(c_allocator) is allocated but dropped when the function
returns null, causing a leak; before returning null set out_len.* = 0 and free
the allocated slice via the same allocator (call c_allocator.free(result.ptr) or
the appropriate allocator.free for result.ptr) so the buffer is released; ensure
behavior matches zpdf_extract_all_reading_order and zpdf_extract_page_markdown
(or alternatively return result.ptr even for zero-length like those functions)
and retain the existing out_len handling and null return semantics used by
zpdf_free_buffer.
Currently WebAssembly build does not support Markdown export. I made this function available in WAPI.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.