Skip to content

Fix remove_markup leaking footnote text on a nested self-closing tag (#3520) - #3653

Open
vineethsaivs wants to merge 1 commit into
piskvorky:developfrom
vineethsaivs:fix/wikicorpus-ref-nested-selfclosing
Open

Fix remove_markup leaking footnote text on a nested self-closing tag (#3520)#3653
vineethsaivs wants to merge 1 commit into
piskvorky:developfrom
vineethsaivs:fix/wikicorpus-ref-nested-selfclosing

Conversation

@vineethsaivs

Copy link
Copy Markdown

Fixes #3520.

filter_wiki left footnote text in its output when a <ref>...</ref> block contained a nested self-closing tag:

from gensim.corpora.wikicorpus import filter_wiki
filter_wiki("total<ref>note<nowiki/>https://example.com/x</ref>end")
# before: 'totalhttps://example.com/xend'
# after:  'totalend'

RE_P1 was r'<ref([> ].*?)(</ref>|/>)'. The /> alternative was meant to match a self-closing ref tag, but with the non-greedy .*? it matched the /> of a nested self-closing tag (<nowiki/>, <br/>) inside the footnote, ending the match early and leaving the rest of the footnote (often a URL) in the text.

The new pattern matches either a self-closing ref (<ref .../>) or a paired ref terminated only by </ref>:

RE_P1 = re.compile(r'<ref(\s[^>]*?)?/>|<ref(\s[^>]*?)?>.*?</ref>', re.DOTALL | re.UNICODE)

Nested self-closing tags inside the footnote no longer cut the match short. Self-closing refs (<ref name="r"/>) and plain paired refs are still removed, and the tag-name guard still avoids matching unrelated tags such as <references/>.

Added a regression test (TestWikiCorpus.test_filter_wiki_ref_with_nested_self_closing_tag) that fails on develop and passes with this change.


Disclosure: prepared with AI assistance; reviewed by me and I can explain every line.

RE_P1 matched `<ref...>...(</ref>|/>)`, so a `<ref>...</ref>` footnote that
contained a nested self-closing tag (e.g. `<nowiki/>` or `<br/>`) terminated the
match at the nested tag's `/>`, leaving the rest of the footnote (often a URL)
in the output of filter_wiki.

Match either a self-closing ref (`<ref .../>`) or a paired ref terminated only
by `</ref>`, so nested self-closing tags inside the footnote no longer cut the
match short. The tag-name guard still avoids matching tags like `<references/>`.

Fixes piskvorky#3520

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qorexdevs

Copy link
Copy Markdown

nice, this is cleaner than trying to patch the old non-greedy /> alternative. splitting into a self-closing branch and a paired branch is the right call.

ran it against a few edge cases and they all hold:

  • total<ref>note<nowiki/>...</ref>end -> totalend (the bug about remove_markup #3520 case)
  • a<ref name="a/b">t</ref>y -> ay (slash inside an attr value doesn't trip the self-closing branch since [^>]*? can't cross >)
  • <references/> and <reference>... stay untouched, same as before, so no new false matches on ref-prefixed tags.

lgtm.

@vineethsaivs

Copy link
Copy Markdown
Author

Thanks for the thorough review and for checking those edge cases, especially the attribute-value slash (name="a/b") not tripping the self-closing branch. That was the exact case I wanted to be sure [^>]*? couldn't cross a > on. Appreciate the LGTM.

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.

bug about remove_markup

2 participants