feat: support section-scoped total page counts - #599
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPage counters now support document-wide or section-scoped totals. The renderer emits scope metadata, and client-side pagination uses reset markers and starting values to calculate section totals while preserving document totals by default. ChangesPage number scoping
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant totalPages
participant PageCounterRenderer
participant updateTotalPageNumbers
totalPages->>PageCounterRenderer: provide DOCUMENT or SECTION scope
PageCounterRenderer->>updateTotalPageNumbers: emit scoped total counter metadata
updateTotalPageNumbers->>updateTotalPageNumbers: calculate totals from reset markers
updateTotalPageNumbers->>PageCounterRenderer: write section or document total
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@quarkdown-html/src/main/typescript/document/handlers/page-numbers.ts`:
- Around line 47-87: Update the section-total calculation around the flush
function to use the section’s reset marker start value: compute (start - 1) +
currentSection.length, defaulting to the existing behavior when no applicable
start value exists, and assign that total to every page in the section. Also
normalize indentation for the pages.forEach and total-page-number update blocks
without changing their behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 52cd7b00-23a3-45b7-a9fe-41bf0a31c12c
📒 Files selected for processing (2)
quarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/node/QuarkdownHtmlNodeRenderer.ktquarkdown-html/src/main/typescript/document/handlers/page-numbers.ts
iamgio
left a comment
iamgio
left a comment
There was a problem hiding this comment.
I'll be unavailable till Sunday 26 and won't be able to look at it closely until then -- sorry about the bad timing. I left some nitpick comments. Also, please add an end-to-end test case.
| @Name("totalpages") | ||
| fun totalPages() = PageCounter(PageCounter.Target.TOTAL).wrappedAsValue() | ||
| fun totalPages( | ||
| @Name("scope") scope: String = "document", |
There was a problem hiding this comment.
@Nameis redundant here- You can directly make it
scope: PageCounter.Scope = PageCounter.Scope.DOCUMENTskipping the string conversion - Please document the new param
| */ | ||
| class PageCounter( | ||
| val target: Target, | ||
| val scope: Scope = Scope.DOCUMENT, |
2 participants
Summary
This PR adds support for section-scoped total page counts through an optional
document | sectionargument on.totalpages.By default,
.totalpagescontinues to return the total number of pages in the document, preserving backwards compatibility.When
sectionis specified, the total page count is calculated from the most recent.resetpagenumber(or from the start of the document if no reset exists), matching the requested behavior.Changes
document | sectionscope to.totalpagesdocumentas the default behaviorTesting
npm test)Closes #596
Summary by CodeRabbit
New Features
totalPages(scope)API option.Bug Fixes
Tests