From 8d99b4f02bff75cdeb61a8cd694789e21428b907 Mon Sep 17 00:00:00 2001 From: yi111 <153097222+Yi-111-a@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:13:19 +0800 Subject: [PATCH] fix(filters): separate each match on its own line for source: watches (#4492) * fix(filters): separate each match on its own line for source: watches #4477 A `source:` watch passes append_pretty_line_formatting=False to the filter helpers, and then keeps the filtered text verbatim instead of running it through html_to_text(). TEXT_FILTER_LIST_LINE_SUFFIX ('
') only works because Inscriptis turns it into a newline, so for that path no separator was emitted at all and every match ran into the next on a single line. Add filter_match_separator(), which picks '\n' instead of '
' when the result will not be passed through Inscriptis, and use it from xpath_filter(), xpath1_filter() and include_filters() - all three carried the same condition. The first match still never gets a separator, so no leading newline is introduced, and the Inscriptis path is byte-for-byte unchanged. * fix(filters): don't double-space xpath element matches on source: watches xpath_filter/xpath1_filter serialise whole elements with etree.tostring(pretty_print=True), which already terminates every match with a newline. Adding the new source: separator unconditionally put a second newline between matched elements, so each one landed on its own line separated by a blank line. Only emit the separator when the accumulated output does not already end on a line break, which leaves attribute and text() matches (plain strings, no newline of their own) separated as before. Found while producing the before/after output requested on #4492. --------- Co-authored-by: Yi-111-a Co-authored-by: Yi-111-a <41898262+github-actions[bot]@users.noreply.github.com> --- changedetectionio/html_tools.py | 45 +++++- .../tests/unit/test_filter_match_separator.py | 148 ++++++++++++++++++ 2 files changed, 187 insertions(+), 6 deletions(-) create mode 100644 changedetectionio/tests/unit/test_filter_match_separator.py diff --git a/changedetectionio/html_tools.py b/changedetectionio/html_tools.py index 83ace3e25..9abeeb06c 100644 --- a/changedetectionio/html_tools.py +++ b/changedetectionio/html_tools.py @@ -95,6 +95,10 @@ TEXT_FILTER_LIST_LINE_SUFFIX = "
" TRANSLATE_WHITESPACE_TABLE = str.maketrans('', '', '\r\n\t ') PERL_STYLE_REGEX = r'^/(.*?)/([a-z]*)?$' +# 'br', 'hr', 'div' and 'p' already make Inscriptis break the line, so a filter match on one of +# them is never suffixed with TEXT_FILTER_LIST_LINE_SUFFIX (a div becomes 4 whitespaces). +FILTER_TAGS_WITH_OWN_NEWLINE = ('br', 'hr', 'div', 'p') + # Whitespace is allowed after the tag name ("") but not after "<" - "< title>" is text, not a tag TITLE_RE = re.compile(r"]*>(.*?)", re.I | re.S) # Locate the tag with pos/endpos instead of data.lower(), which copies the whole document (67MB seen @@ -237,6 +241,38 @@ def perl_style_slash_enclosed_regex_to_options(regex): return regex +def filter_match_separator(append_pretty_line_formatting, html_block, tag): + """What to append after a filter match so that each match gets its own line in the diff. + + Always empty for the first match, so no leading separator is ever emitted. + + :param append_pretty_line_formatting: True while the result still has to survive + html_to_text() (Inscriptis). `source:` watches pass False - see processor.py, + `append_pretty_line_formatting=not self.watch.is_source_type_url`. + :param html_block: the output accumulated so far, used to skip the first match + :param tag: the matched element's tag name, or None when the match is a plain string + (an attribute or text() node) and so is not HTML at all + """ + if not append_pretty_line_formatting: + # A `source:` watch keeps the filtered text verbatim and never runs html_to_text(), + # so a literal '
' would reach the snapshot as visible text. A real newline is + # the only separator that survives there - and with no separator at all every match + # ran straight into the next on a single line, see #4477. + if not len(html_block): + return "" + + # xpath_filter/xpath1_filter serialise whole elements with + # etree.tostring(pretty_print=True), which already ends every match with a newline. + # Adding one here as well would double-space those matches, so only separate when + # the accumulated output does not already end on a line break. Attribute and + # text() matches return plain strings and do need the separator. + return "" if html_block.endswith("\n") else "\n" + + if not len(html_block) or tag in FILTER_TAGS_WITH_OWN_NEWLINE: + return "" + + return TEXT_FILTER_LIST_LINE_SUFFIX + # Given a CSS Rule, and a blob of HTML, return the blob of HTML that matches def include_filters(include_filters, html_content, append_pretty_line_formatting=False): from bs4 import BeautifulSoup @@ -249,8 +285,7 @@ def include_filters(include_filters, html_content, append_pretty_line_formatting # And where the matched result doesn't include something that will cause Inscriptis to add a newline # (This way each 'match' reliably has a new-line in the diff) # Divs are converted to 4 whitespaces by inscriptis - if append_pretty_line_formatting and len(html_block) and not element.name in (['br', 'hr', 'div', 'p']): - html_block += TEXT_FILTER_LIST_LINE_SUFFIX + html_block += filter_match_separator(append_pretty_line_formatting, html_block, getattr(element, 'name', None)) html_block += str(element) @@ -422,8 +457,7 @@ def xpath_filter(xpath_filter, html_content, append_pretty_line_formatting=False # And where the matched result doesn't include something that will cause Inscriptis to add a newline # (This way each 'match' reliably has a new-line in the diff) # Divs are converted to 4 whitespaces by inscriptis - if append_pretty_line_formatting and len(html_block) and (not hasattr( element, 'tag' ) or not element.tag in (['br', 'hr', 'div', 'p'])): - html_block += TEXT_FILTER_LIST_LINE_SUFFIX + html_block += filter_match_separator(append_pretty_line_formatting, html_block, getattr(element, 'tag', None)) if type(element) == str: html_block += element @@ -483,8 +517,7 @@ def xpath1_filter(xpath_filter, html_content, append_pretty_line_formatting=Fals # And where the matched result doesn't include something that will cause Inscriptis to add a newline # (This way each 'match' reliably has a new-line in the diff) # Divs are converted to 4 whitespaces by inscriptis - if append_pretty_line_formatting and len(html_block) and (not hasattr(element, 'tag') or not element.tag in (['br', 'hr', 'div', 'p'])): - html_block += TEXT_FILTER_LIST_LINE_SUFFIX + html_block += filter_match_separator(append_pretty_line_formatting, html_block, getattr(element, 'tag', None)) # Some kind of text, UTF-8 or other if isinstance(element, (str, bytes)): diff --git a/changedetectionio/tests/unit/test_filter_match_separator.py b/changedetectionio/tests/unit/test_filter_match_separator.py new file mode 100644 index 000000000..8668936b8 --- /dev/null +++ b/changedetectionio/tests/unit/test_filter_match_separator.py @@ -0,0 +1,148 @@ +#!/usr/bin/env python3 +import unittest + +from changedetectionio import html_tools + +# A page whose elements give five distinct href attribute values, so a multi-match +# filter has something to separate. Paraphrased from #4477. +LINK_PAGE = """ + + + + + +Page body""" + +HREFS = ['/static/apple-touch/wikipedia.png', + '/static/favicon/wikipedia.ico', + '//creativecommons.org/licenses/by-sa/4.0/', + '//upload.wikimedia.org/style.css', + 'https://wikis.world/@wikipedia'] + + +class TestSourceTypeFilterSeparator(unittest.TestCase): + """#4477: a `source:` watch runs an XPath/CSS filter over more than one match. + + processor.py passes append_pretty_line_formatting=not self.watch.is_source_type_url, and + for a `source:` watch the filtered text is then kept verbatim instead of going through + html_to_text(). So TEXT_FILTER_LIST_LINE_SUFFIX ('
') cannot be used - it would show + up as literal text - and the previous code added no separator at all, which ran every + match into the next on a single line. + """ + + def test_xpath_multiple_matches_are_one_per_line(self): + filtered = html_tools.xpath_filter( + xpath_filter='//link/@href', + html_content=LINK_PAGE, + append_pretty_line_formatting=False, + ) + + self.assertNotIn('
', filtered, "'
' would leak into a source: snapshot as text") + self.assertEqual(HREFS, filtered.splitlines()) + + def test_xpath1_multiple_matches_are_one_per_line(self): + filtered = html_tools.xpath1_filter( + xpath_filter='//link/@href', + html_content=LINK_PAGE, + append_pretty_line_formatting=False, + ) + + self.assertNotIn('
', filtered) + self.assertEqual(HREFS, filtered.splitlines()) + + def test_css_multiple_matches_are_one_per_line(self): + filtered = html_tools.include_filters( + include_filters='link', + html_content=LINK_PAGE, + append_pretty_line_formatting=False, + ) + + self.assertNotIn('
', filtered) + self.assertEqual(5, len(filtered.splitlines())) + for href in HREFS: + self.assertIn(href, filtered) + + def test_single_match_gets_no_separator(self): + """One match means there is nothing to separate, so no leading newline either.""" + filtered = html_tools.xpath_filter( + xpath_filter='//link[@rel="icon"]/@href', + html_content=LINK_PAGE, + append_pretty_line_formatting=False, + ) + + self.assertEqual('/static/favicon/wikipedia.ico', filtered) + + def test_no_matches_gets_empty_output(self): + filtered = html_tools.xpath_filter( + xpath_filter='//link[@rel="does-not-exist"]/@href', + html_content=LINK_PAGE, + append_pretty_line_formatting=False, + ) + + self.assertEqual('', filtered) + + def test_xpath_element_matches_are_not_double_spaced(self): + """xpath_filter serialises elements with etree.tostring(pretty_print=True), which + already ends each match with a newline. The separator must not add a second one, + which would leave every matched element separated by a blank line. + + The page is written without whitespace between the elements on purpose: the + newline between them in LINK_PAGE is the element's own tail, and tostring emits it, + so the output is single-spaced regardless of which separator is used.""" + compact = '' + ''.join( + f'' for href in HREFS) + '' + + for filtered in ( + html_tools.xpath_filter(xpath_filter='//link', html_content=compact, + append_pretty_line_formatting=False), + html_tools.xpath1_filter(xpath_filter='//link', html_content=compact, + append_pretty_line_formatting=False), + ): + self.assertNotIn('
', filtered) + self.assertNotIn('\n\n', filtered, 'matched elements must not be double-spaced') + self.assertEqual(5, len(filtered.splitlines())) + for href in HREFS: + self.assertIn(href, filtered) + + def test_xpath_element_matches_match_the_pre_fix_spacing(self): + """Element matches must come out exactly as they did before this branch, i.e. the + separator is a no-op for them and only attribute/text() matches are affected.""" + compact = '' + ''.join( + f'' for href in HREFS) + '' + + filtered = html_tools.xpath_filter(xpath_filter='//link', html_content=compact, + append_pretty_line_formatting=False) + self.assertEqual('\n'.join(f'' for href in HREFS) + '\n', + filtered) + + +class TestFilterSeparatorHelper(unittest.TestCase): + """The separator decision itself, including the unchanged Inscriptis path.""" + + def test_first_match_never_gets_a_separator(self): + for pretty in (True, False): + self.assertEqual('', html_tools.filter_match_separator(pretty, "", None)) + self.assertEqual('', html_tools.filter_match_separator(pretty, "", 'div')) + + def test_pretty_path_keeps_using_the_br_marker(self): + self.assertEqual(html_tools.TEXT_FILTER_LIST_LINE_SUFFIX, + html_tools.filter_match_separator(True, 'something', None)) + + def test_pretty_path_leaves_tags_that_break_a_line_alone(self): + for tag in html_tools.FILTER_TAGS_WITH_OWN_NEWLINE: + self.assertEqual('', html_tools.filter_match_separator(True, 'something', tag)) + + def test_non_pretty_path_uses_a_newline(self): + self.assertEqual("\n", html_tools.filter_match_separator(False, 'something', None)) + # Even for tags that would break a line under Inscriptis - it never runs here, and a + # '
' is not a newline in the verbatim text. + self.assertEqual("\n", html_tools.filter_match_separator(False, 'something', 'div')) + + def test_non_pretty_path_adds_nothing_when_output_already_ends_on_a_newline(self): + """xpath element matches are already newline-terminated by etree.tostring.""" + self.assertEqual('', html_tools.filter_match_separator(False, '\n', 'link')) + self.assertEqual('', html_tools.filter_match_separator(False, '\n', None)) + + +if __name__ == '__main__': + unittest.main()