Skip to content

feat(core/render-biblio): render pages field#5252

Open
marcoscaceres wants to merge 2 commits into
mainfrom
feat/4067-biblio-pages
Open

feat(core/render-biblio): render pages field#5252
marcoscaceres wants to merge 2 commits into
mainfrom
feat/4067-biblio-pages

Conversation

@marcoscaceres
Copy link
Copy Markdown
Contributor

Closes #4067

Copy link
Copy Markdown
Contributor

Copilot AI left a 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 extends bibliography rendering so localBiblio/biblio entries can include a pages field in rendered references, alongside updating typing and adding a focused spec. It fits into ReSpec’s core reference pipeline by changing how core/render-biblio serializes bibliography entries for the generated References section.

Changes:

  • Add pages to the BiblioData type so bibliography entries can carry page information.
  • Render pages in core/render-biblio output and add a small French localization block in the same module.
  • Extend tests/spec/core/biblio-spec.js with a new local bibliography entry and an assertion for page rendering.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File Description
tests/spec/core/biblio-spec.js Adds a new local biblio fixture and a spec intended to verify page rendering.
src/type-helper.d.ts Extends the bibliography entry type with an optional pages field.
src/core/render-biblio.js Updates bibliography serialization to emit page info and adds French reference strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/render-biblio.js
Comment thread tests/spec/core/biblio-spec.js Outdated
Comment thread src/core/render-biblio.js
@marcoscaceres marcoscaceres force-pushed the feat/4067-biblio-pages branch 2 times, most recently from 0fda851 to bfcf9cb Compare May 4, 2026 07:35
@marcoscaceres marcoscaceres force-pushed the feat/4067-biblio-pages branch from bfcf9cb to 894955a Compare May 4, 2026 07:43
When a publisher is followed by pages, use a comma separator instead of a
period ("Publisher, pp. 369-384" not "Publisher. pp. 369-384").

Remove the now-unused endNormalizer/endWithDot helpers. Strengthen the
pages test to assert the full publisher-pages sequence. Add a French
locale test for the reference_not_found string.
@marcoscaceres
Copy link
Copy Markdown
Contributor Author

@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed?

@marcoscaceres
Copy link
Copy Markdown
Contributor Author

@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed?

Copy link
Copy Markdown
Contributor

Copilot AI commented May 8, 2026

@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed?

Yes, all three items are addressed in commit 6aa8693:

  1. Publisher/pages punctuation — the code now strips any trailing period from the publisher and uses a comma when pages is present, producing Publisher Name, pp. 369-384. as expected.
  2. Test specificity — the test now asserts the full "Publisher Pages, pp. 369-384" sequence, catching both wrong punctuation and wrong placement.
  3. French locale test — a new test sets lang: "fr", references a missing entry, and asserts the #references-informatives section ID and "Référence non trouvée." error text, mirroring the Spanish locale test.

@marcoscaceres marcoscaceres marked this pull request as ready for review May 9, 2026 02:40
@marcoscaceres marcoscaceres requested review from sidvishnoi and removed request for sidvishnoi May 9, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

include the 'pages' field of specref entries in references

3 participants