Skip to content

Conversation

@pmachapman
Copy link
Collaborator

@pmachapman pmachapman commented Nov 4, 2025

This PR adds support for the links with titles, which in Paratext are generated as <link> elements, not the usual <char> element that links without titles are created as. These are used in footnotes and introductions of some translations, particularly the NIV2011.


This change is Reviewable

@pmachapman pmachapman added the will require testing PR should not be merged until testers confirm testing is complete label Nov 4, 2025
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.91%. Comparing base (12c8136) to head (5b8787c).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3557      +/-   ##
==========================================
+ Coverage   82.90%   82.91%   +0.01%     
==========================================
  Files         605      605              
  Lines       36974    36996      +22     
  Branches     6058     6038      -20     
==========================================
+ Hits        30652    30675      +23     
  Misses       5408     5408              
+ Partials      914      913       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working well in my testing. Send receive seems to accurately get the content if edits are made around the new link blot. I followed the acceptance tests and could not find any issues

@RaymondLuong3 reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pmachapman)


src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/quill-editor-registration/quill-formats/quill-blots.spec.ts line 366 at r1 (raw file):

    });

    it('should set title from contents ignoring an link with  no text', () => {

Nit: typo

Code quote:

ignoring an link 

src/SIL.XForge.Scripture/usx-sf.rnc line 433 at r1 (raw file):

char.link =
    attribute link-href { xsd:string
        { pattern = "(.*///?(.*/?)+)|((prj:[A-Za-z\-0-9]{3,8} )?[A-Z1-4]{3} \d+:\d+(\-\d+)?[ ]*)|(#[^\s]+)" } }?, # The resource being linked to as a URI

Why is this needed? Are there sometimes whitespace at the end of the href?

Code quote:

?[ ]*)

@RaymondLuong3 RaymondLuong3 added ready to test and removed will require testing PR should not be merged until testers confirm testing is complete labels Nov 6, 2025
Copy link
Collaborator Author

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 8 of 9 files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)


src/SIL.XForge.Scripture/usx-sf.rnc line 433 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Why is this needed? Are there sometimes whitespace at the end of the href?

Yes. Because the USFM is in the format \+xt see verse 20|GEN 2:20\+xt*, sometimes users when typing this out in Paratext will leave a space at the end, i.e. \+xt see verse 20|GEN 2:20 \+xt*. PT has no issue using the cross reference in this case, so I thought the best way is just to allow it.


src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/quill-editor-registration/quill-formats/quill-blots.spec.ts line 366 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: typo

Done. Thanks!

@RaymondLuong3 RaymondLuong3 merged commit 03bf6db into master Nov 12, 2025
22 of 23 checks passed
@RaymondLuong3 RaymondLuong3 deleted the fix/SF-3629 branch November 12, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants