From 3d9d6987935c90e948deb4abff457f4681dc303d Mon Sep 17 00:00:00 2001 From: Travis Abendshien <46939827+CyanVoxel@users.noreply.github.com> Date: Sun, 4 Oct 2026 12:11:04 -0700 Subject: [PATCH] refactor: align wcmatch and ripgrep behavior --- docs/ignore.md | 19 +- .../core/library/alchemy/migrations.py | 24 ++ .../alchemy/registries/ignored_registry.py | 5 +- src/tagstudio/core/library/ignore.py | 151 +++++----- src/tagstudio/core/library/scanners.py | 9 +- src/tagstudio/core/library/sync.py | 13 +- src/tagstudio/previews/file_renderer.py | 8 +- src/tagstudio/qt/mixed/file_attributes.py | 2 +- src/tagstudio/qt/mixed/migration_modal.py | 22 +- tests/conftest.py | 10 + tests/core/library/test_ignore.py | 285 ++++++++++++++---- tests/core/library/test_migrations.py | 29 +- tests/core/library/test_sync.py | 40 +++ 13 files changed, 428 insertions(+), 189 deletions(-) diff --git a/docs/ignore.md b/docs/ignore.md index 43a9f6fa53..2bf2818060 100644 --- a/docs/ignore.md +++ b/docs/ignore.md @@ -49,10 +49,6 @@ Minecraft/Website !!! note "" _This section sourced and adapted from Git's[^1] `.gitignore` [documentation](https://git-scm.com/docs/gitignore)._ -### Internal Processes - -When scanning your library directories, the `.ts_ignore` file is read by either the [`wcmatch`](https://facelessuser.github.io/wcmatch/glob/) library or [`ripgrep`](https://github.com/BurntSushi/ripgrep) in glob mode depending if you have the later installed on your system and it's detected by TagStudio. Ripgrep is the preferred method for scanning directories due to its improved performance and identical pattern matching to `.gitignore`. This mixture of tools may lead to slight inconsistencies if not using `ripgrep`. - --- ### Comments ( `#` ) @@ -142,10 +138,6 @@ A `!` prefix before a pattern negates the pattern, allowing any files matched ma ``` - -!!! bug "Directory Exclusion Negation" - TagStudio attempts to match the behavior of a `.gitignore` file 1:1, however if you don't have `ripgrep` installed on your system and TagStudio falls back to its internal pattern matcher, excluded directories can be overwritten by further negations, unlike `.gitignore` behavior. Be wary that this is **not officially supported**, and this behavior may be removed at any time. - --- ### Wildcards @@ -275,18 +267,22 @@ Character sets and ranges are specific and powerful forms of wildcards that use ``` === "Ignore all files EXCEPT .jpg files" ```toml + # Ignore everything to start, + # reinclude subfolders, + # then reinclude .jpg files located anywhere * + !*/ !*.jpg ``` === "Ignore all .jpg files in specific folders" ```toml - ./Photos/Worst Vacation/*.jpg + Photos/Worst Vacation/*.jpg Music/Artwork Art/*.jpg ``` !!! tip "Ensuring Complete Extension Matches" - For some filetypes, it may be nessisary to specify different casing and alternative spellings in order to match with all possible variations of an extension in your library. + For some filetypes, it may be necessary to specify different casing and alternative spellings in order to match with all possible variations of an extension in your library. ```toml title="Ignore (Most) Possible JPEG File Extensions" # The JPEG Cinematic Universe @@ -305,7 +301,8 @@ Character sets and ranges are specific and powerful forms of wildcards that use === "Ignore all "Cache" folders" ```toml - # Matches any folder called "Cache" no matter where it is in your library. + # Matches any folder called "cache"/"Cache" no matter where it is in your library. + Cache/ cache/ ``` === "Ignore a "Downloads" folder" diff --git a/src/tagstudio/core/library/alchemy/migrations.py b/src/tagstudio/core/library/alchemy/migrations.py index 93ce11229b..3c62eb0785 100644 --- a/src/tagstudio/core/library/alchemy/migrations.py +++ b/src/tagstudio/core/library/alchemy/migrations.py @@ -662,3 +662,27 @@ def run(cls, conn: Connection, library_dir: Path, fmt_log: LoggingMethod): "suffix = :suffix WHERE id = :id", updates, ) + + logger.info(fmt_log("Repairing 'reinclude folders' pattern in the .ts_ignore file...")) + cls._repair_reinclude_folders_pattern(library_dir) + + @classmethod + def _repair_reinclude_folders_pattern(cls, library_dir: Path): + """Add "!*/" following "*" lines in the `.ts_ignore` if one isn't already present. + + Under wcmatch's rules paths could be reincluded after a "*" without requiring a + "!*/" after it, unlike the desired `.gitignore`-type behavior. + """ + ts_ignore = library_dir / TS_FOLDER_NAME / IGNORE_NAME + if not ts_ignore.exists(): + return + + # If "!*/" already follows "*", do nothing and return + lines = ts_ignore.read_text(encoding="utf8").splitlines() + patterns = [line.rstrip() for line in lines] + if "*" not in patterns or "!*/" in patterns: + return + + # Add "!*/" after the first "*" found, if any + lines.insert(patterns.index("*") + 1, "!*/") + ts_ignore.write_text("\n".join(lines) + "\n", encoding="utf8") diff --git a/src/tagstudio/core/library/alchemy/registries/ignored_registry.py b/src/tagstudio/core/library/alchemy/registries/ignored_registry.py index 5afa6636df..1e6fd3d764 100644 --- a/src/tagstudio/core/library/alchemy/registries/ignored_registry.py +++ b/src/tagstudio/core/library/alchemy/registries/ignored_registry.py @@ -36,10 +36,7 @@ def refresh_ignored_entries(self) -> Iterator[int]: for i, entry in enumerate(self.lib.all_entries()): yield i - if not Ignore.compiled_patterns: - # If the compiled_patterns has malfunctioned, don't consider that a false positive - yield i - elif Ignore.compiled_patterns.match(entry.path): + if Ignore.matcher.is_ignored(entry.path): self.ignored_entries.append(entry) def remove_ignored_entries(self) -> None: diff --git a/src/tagstudio/core/library/ignore.py b/src/tagstudio/core/library/ignore.py index 8ff8ce17ff..3bf9dfc239 100644 --- a/src/tagstudio/core/library/ignore.py +++ b/src/tagstudio/core/library/ignore.py @@ -3,6 +3,7 @@ from pathlib import Path +from typing import NamedTuple import structlog from wcmatch import glob @@ -12,12 +13,10 @@ logger = structlog.get_logger() -PATH_GLOB_FLAGS: int = glob.GLOBSTARLONG | glob.DOTGLOB | glob.NEGATE +_RULE_FLAGS: int = glob.GLOBSTAR | glob.DOTGLOB GLOBAL_IGNORE = [ - # TagStudio ------------------- - f"{TS_FOLDER_NAME}", # Trash ----------------------- ".Trash-*", ".Trash", @@ -35,65 +34,52 @@ ] -def ignore_to_glob(ignore_patterns: list[str]) -> list[str]: - """Convert .gitignore-like patterns to Unix-like glob syntax. - - Args: - ignore_patterns (list[str]): The .gitignore-like patterns to convert. - """ - glob_patterns: list[str] = list(ignore_patterns) - glob_patterns_remove: list[str] = [] - additional_patterns: list[str] = [] - root_patterns: list[str] = [] - - # Expand .gitignore patterns to mimic the same behavior with unix-like glob patterns. - for pattern in glob_patterns: - # Temporarily remove any exclusion character before processing - exclusion_char = "" - gp = pattern - if pattern.startswith("!"): - gp = pattern[1:] - exclusion_char = "!" - - if not gp.startswith("**/") and not gp.startswith("*/") and not gp.startswith("/"): - # Create a version of a prefix-less pattern that starts with "**/" - gp = "**/" + gp - additional_patterns.append(exclusion_char + gp) - - gp = gp.removeprefix("**/").removeprefix("*/") - additional_patterns.append(exclusion_char + gp) - - elif gp.startswith("/"): - # Matches "/file" case for .gitignore behavior where it should only match - # a file or folder in the root directory and nowhere else. - glob_patterns_remove.append(pattern) - gp = gp.lstrip("/") - root_patterns.append(exclusion_char + gp) - - remove_set = set(glob_patterns_remove) - glob_patterns = [p for p in glob_patterns if p not in remove_set] - # root_patterns must be merged in before the "/**" suffix pass below, otherwise a rooted - # directory pattern (e.g. "/Downloads/") never gets a "/**" variant and matches nothing. - glob_patterns = glob_patterns + additional_patterns + root_patterns - - # Add "/**" suffix to suffix-less patterns to match implicit .gitignore behavior. - for pattern in list(glob_patterns): - if pattern.endswith("/**"): - continue - - glob_patterns.append(pattern.removesuffix("/*").removesuffix("/") + "/**") - - # Fix wcmatch interpreting "**" as "one or more" to be a .gitignore style "zero or more". - # Otherwise "**/foo" won't match a root "foo" and "a/**/b" won't match match "a/b". - for pattern in list(glob_patterns): - collapsed = pattern.removeprefix("**/").replace("/**/", "/") - if collapsed != pattern: - glob_patterns.append(collapsed) - - glob_patterns = list(dict.fromkeys(glob_patterns)) # Ordered deduplication - - logger.info("[Ignore]", glob_patterns=glob_patterns) - return glob_patterns +class _Rule(NamedTuple): + matcher: glob.WcMatcher + negated: bool + dir_only: bool + name_only: bool + + +def _parse_rule(pattern: str) -> _Rule | None: + negated = pattern.startswith("!") + pattern = pattern.removeprefix("!") + dir_only = pattern.endswith("/") + pattern = pattern.rstrip("/") + if not pattern: + return None + # A slashed pattern is relative to the root, otherwise it matches any name + name_only = "/" not in pattern + pattern = pattern.removeprefix("/") + return _Rule(glob.compile(pattern, flags=_RULE_FLAGS), negated, dir_only, name_only) + + +class IgnoreMatcher: + """Matches paths relative to the library against .gitignore-style patterns.""" + + def __init__(self, patterns: list[str]) -> None: + self._rules = [rule for pattern in patterns if (rule := _parse_rule(pattern))] + self._folder_verdicts: dict[str, bool] = {} + + def match(self, path: str, is_dir: bool) -> bool: + """Whether `path` is ignored, without checking its parent folders.""" + name = path.rpartition("/")[2] + for rule in reversed(self._rules): + target = name if rule.name_only else path + if (is_dir or not rule.dir_only) and rule.matcher.match(target): + return not rule.negated + return False + + def is_ignored(self, path: Path | str) -> bool: + """Whether the file at `path` is ignored, including by any ignored parent folder.""" + parts = Path(path).as_posix().split("/") + for depth in range(1, len(parts)): + folder = "/".join(parts[:depth]) + if folder not in self._folder_verdicts: + self._folder_verdicts[folder] = self.match(folder, is_dir=True) + if self._folder_verdicts[folder]: + return True + return self.match("/".join(parts), is_dir=False) def migrate_ext_list(exts: list[str], is_exclude_list: bool) -> str: @@ -108,7 +94,7 @@ def migrate_ext_list(exts: list[str], is_exclude_list: bool) -> str: prefix = "" if not is_exclude_list: prefix = "!" - out += "*\n" + out += "*\n!*/\n" out += "\n".join([f"{prefix}*.{x.lstrip('.')}\n" for x in exts]) return out @@ -129,8 +115,8 @@ class Ignore(metaclass=Singleton): """Class for processing and managing glob-like file ignore file patterns.""" _last_loaded: tuple[Path, float] | None = None - _patterns: list[str] = [] - compiled_patterns: glob.WcMatcher | None = None + _patterns: list[str] = [*GLOBAL_IGNORE, TS_FOLDER_NAME] + matcher: IgnoreMatcher = IgnoreMatcher(_patterns) @staticmethod def read_ignore_file(library_dir: Path) -> list[str]: @@ -165,46 +151,57 @@ def write_ignore_file(library_dir: Path, lines: list[str]) -> None: f.writelines(lines) @staticmethod - def get_patterns(library_dir: Path, include_global: bool = True) -> list[str]: + def get_patterns( + library_dir: Path, + include_global: bool = True, + update_state: bool = True, + ) -> list[str]: """Get the ignore patterns for the given library directory. + The library's .TagStudio folder always comes last so it doesn't get reincluded. + Args: library_dir (Path): The path of the library to load patterns from. - include_global (bool): Flag for including the global ignore set. - In most scenarios, this should be True. + include_global (bool): Flag for including the global ignore list. + update_state (bool): Flag for also loading the patterns into the class's state. + Should be True outside of exceptions that may include tests, migrations, etc. """ - patterns = GLOBAL_IGNORE if include_global else [] + global_patterns = GLOBAL_IGNORE if include_global else [] ts_ignore_path = Path(library_dir / TS_FOLDER_NAME / IGNORE_NAME) + # Return computed patterns if the state of the Ignore singleton shouldn't be updated. + if not update_state: + return [*global_patterns, *Ignore._load_ignore_file(ts_ignore_path), TS_FOLDER_NAME] + + # Return default internal patterns if no .ts_ignore exists. if not ts_ignore_path.exists(): logger.info( "[Ignore] No .ts_ignore file found", path=ts_ignore_path, ) Ignore._last_loaded = None - Ignore._patterns = patterns + Ignore._patterns = [*global_patterns, TS_FOLDER_NAME] + Ignore.matcher = IgnoreMatcher(Ignore._patterns) return Ignore._patterns # Process the .ts_ignore file if the previous result is non-existent or outdated. loaded = (ts_ignore_path, ts_ignore_path.stat().st_mtime) - if not Ignore._last_loaded or (Ignore._last_loaded and Ignore._last_loaded != loaded): + if Ignore._last_loaded != loaded: logger.info( "[Ignore] Processing the .ts_ignore file...", library=library_dir, last_mtime=Ignore._last_loaded[1] if Ignore._last_loaded else None, new_mtime=loaded[1], ) - Ignore._patterns = patterns + Ignore._load_ignore_file(ts_ignore_path) - Ignore.compiled_patterns = glob.compile( - patterns=ignore_to_glob(Ignore._patterns), - flags=PATH_GLOB_FLAGS, - ) + user_patterns = Ignore._load_ignore_file(ts_ignore_path) + Ignore._patterns = [*global_patterns, *user_patterns, TS_FOLDER_NAME] + Ignore.matcher = IgnoreMatcher(Ignore._patterns) else: logger.info( "[Ignore] No updates to the .ts_ignore detected", library=library_dir, - last_mtime=Ignore._last_loaded[1], + last_mtime=loaded[1], new_mtime=loaded[1], ) Ignore._last_loaded = loaded diff --git a/src/tagstudio/core/library/scanners.py b/src/tagstudio/core/library/scanners.py index 0f71682496..a56c6aac8a 100644 --- a/src/tagstudio/core/library/scanners.py +++ b/src/tagstudio/core/library/scanners.py @@ -10,10 +10,9 @@ from pathlib import Path import structlog -from wcmatch import glob from tagstudio.core.constants import TS_FOLDER_NAME -from tagstudio.core.library.ignore import PATH_GLOB_FLAGS, ignore_to_glob +from tagstudio.core.library.ignore import IgnoreMatcher from tagstudio.core.utils.ripgrep_status import RipgrepStatus from tagstudio.core.utils.silent_subprocess import silent_popen # pyright: ignore @@ -99,7 +98,7 @@ def _scan_with_ripgrep(scan_dir: Path, ignore_patterns: list[str]) -> Iterator[P def _scan_with_internal_scanner(scan_dir: Path, ignore_patterns: list[str]) -> Iterator[Path]: """Scan for files with the internal scanner.""" logger.info("[Scanners] Using internal scanner for scanning", path=scan_dir) - matcher = glob.compile(patterns=ignore_to_glob(ignore_patterns), flags=PATH_GLOB_FLAGS) + matcher = IgnoreMatcher(ignore_patterns) def walk(dir_path: Path, ancestors: frozenset[str]) -> Iterator[Path]: try: @@ -110,9 +109,9 @@ def walk(dir_path: Path, ancestors: frozenset[str]) -> Iterator[Path]: for item in dir_items: rel = Path(item.path).relative_to(scan_dir) - if matcher.match(rel.as_posix()): - continue try: + if matcher.match(rel.as_posix(), is_dir=item.is_dir()): + continue item_stat = item.stat(follow_symlinks=True) except OSError: continue diff --git a/src/tagstudio/core/library/sync.py b/src/tagstudio/core/library/sync.py index 9b2ca0cdc7..f0a3983cd8 100644 --- a/src/tagstudio/core/library/sync.py +++ b/src/tagstudio/core/library/sync.py @@ -105,16 +105,21 @@ def sync_dir( sleep(0) start_time_loop = time() + # NOTE: Ignored files are skipped during the scan. + if not self.cancelled: + for entry in self.library.get_entries([cache[key] for key in unvisited]): + if self.cancelled: + break + if not (library_dir / entry.path).is_file(): + self.unlinked_entries.append(entry) + if self.cancelled: yield count, len(self.new_paths) logger.info("[Sync] Directory scan cancelled", path=library_dir, files_scanned=count) return - unlinked_ids = {cache[key] for key in unvisited} if self.library.duplicate_path_entry_ids: - unlinked_ids.update(self.library.duplicate_path_entry_ids) - if unlinked_ids: - self.unlinked_entries = self.library.get_entries(list(unlinked_ids)) + self.unlinked_entries += self.library.get_entries(self.library.duplicate_path_entry_ids) if self.unlinked_entries: yield -1, -1 # Signals the UI that repair work is starting diff --git a/src/tagstudio/previews/file_renderer.py b/src/tagstudio/previews/file_renderer.py index 73086bb9aa..acceeeb237 100644 --- a/src/tagstudio/previews/file_renderer.py +++ b/src/tagstudio/previews/file_renderer.py @@ -656,12 +656,8 @@ def fetch_cached_image(file_name: Path): # Check if the file is supposed to be ignored and render an overlay if needed try: - if ( - image - and Ignore.compiled_patterns - and Ignore.compiled_patterns.match( - filepath.relative_to(unwrap(self.lib.library_dir)) - ) + if image and Ignore.matcher.is_ignored( + filepath.relative_to(unwrap(self.lib.library_dir)) ): image = render_ignored((scaled_size, scaled_size), image) except TypeError: diff --git a/src/tagstudio/qt/mixed/file_attributes.py b/src/tagstudio/qt/mixed/file_attributes.py index b4136fd0ec..6468fb3290 100644 --- a/src/tagstudio/qt/mixed/file_attributes.py +++ b/src/tagstudio/qt/mixed/file_attributes.py @@ -190,7 +190,7 @@ def add_newline(stats_label_text: str) -> str: red = get_ui_color(ColorType.PRIMARY, UiColor.RED) orange = get_ui_color(ColorType.PRIMARY, UiColor.ORANGE) - if Ignore.compiled_patterns and Ignore.compiled_patterns.match( + if Ignore.matcher.is_ignored( filepath.relative_to(unwrap(self.library.library_dir)) ): stats_label_text = ( diff --git a/src/tagstudio/qt/mixed/migration_modal.py b/src/tagstudio/qt/mixed/migration_modal.py index 3f0829c9f2..e9072e90dd 100644 --- a/src/tagstudio/qt/mixed/migration_modal.py +++ b/src/tagstudio/qt/mixed/migration_modal.py @@ -23,10 +23,8 @@ ) from sqlalchemy import select from sqlalchemy.orm import Session -from wcmatch import glob from tagstudio.core.constants import ( - IGNORE_NAME, LEGACY_TAG_FIELD_IDS, TAG_ARCHIVED, TAG_FAVORITE, @@ -39,7 +37,7 @@ from tagstudio.core.library.alchemy.joins import TagParent from tagstudio.core.library.alchemy.library import Library as SqliteLibrary from tagstudio.core.library.alchemy.models import Entry, TagAlias -from tagstudio.core.library.ignore import PATH_GLOB_FLAGS, Ignore, ignore_to_glob +from tagstudio.core.library.ignore import Ignore, IgnoreMatcher from tagstudio.core.library.json.library import Library as JsonLibrary from tagstudio.core.library.json.library import Tag as JsonTag from tagstudio.core.utils.types import unwrap @@ -512,18 +510,14 @@ def color_value_conditional(self, old_value: int | str, new_value: int | str) -> return str(f"{new_value}") def assert_ignore_parity(self) -> None: - compiled_pats = glob.compile( - ignore_to_glob( - Ignore._load_ignore_file( # pyright: ignore[reportPrivateUsage] - unwrap(self.json_lib.library_dir) / TS_FOLDER_NAME / IGNORE_NAME - ) - ), - flags=PATH_GLOB_FLAGS, - ) # copied from Ignore.get_patterns since that method modifies singleton state - path = self.json_lib.library_dir / "filename" + library_dir = unwrap(self.json_lib.library_dir) + matcher = IgnoreMatcher(Ignore.get_patterns(library_dir, update_state=False)) + is_exclude_list = self.json_lib.is_exclude_list for ext in self.json_lib.ext_list: - assert compiled_pats.match(str(path / ext)) == self.json_lib.is_exclude_list - assert compiled_pats.match(str(path / ".not_a_real_ext")) != self.json_lib.is_exclude_list + filename = f"file.{ext.lstrip('.')}" + assert matcher.is_ignored(filename) == is_exclude_list + assert matcher.is_ignored(f"folder/{filename}") == is_exclude_list + assert matcher.is_ignored("folder/file.not_a_real_ext") != is_exclude_list def check_ignore_parity(self) -> bool: try: diff --git a/tests/conftest.py b/tests/conftest.py index 71bb86c7e4..9f91de585b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,6 +25,7 @@ from tagstudio.core.constants import THUMB_CACHE_NAME, TS_FOLDER_NAME from tagstudio.core.library.alchemy.library import Library from tagstudio.core.library.alchemy.models import Entry, Tag +from tagstudio.core.library.ignore import GLOBAL_IGNORE, Ignore, IgnoreMatcher from tagstudio.qt.qt_driver import QtDriver from tagstudio.qt.views.layouts.thumb_grid_layout import ThumbGridLayout @@ -171,6 +172,15 @@ def _reset_media_types(): assert pre_snapshop == MediaTypes._snapshot(), "The MediaTypes state was not restored!" +@pytest.fixture(autouse=True) +def _reset_ignore(): + """Reset the Ignore state after each test.""" + yield + Ignore._last_loaded = None + Ignore._patterns = [*GLOBAL_IGNORE, TS_FOLDER_NAME] + Ignore.matcher = IgnoreMatcher(Ignore._patterns) + + @pytest.fixture def qt_driver(library: Library, library_dir: Path): class Args: diff --git a/tests/core/library/test_ignore.py b/tests/core/library/test_ignore.py index fcd939bce6..7be669cc2a 100644 --- a/tests/core/library/test_ignore.py +++ b/tests/core/library/test_ignore.py @@ -3,109 +3,100 @@ # pyright: reportPrivateUsage=false +from collections.abc import Iterable from pathlib import Path -from wcmatch import glob +import pytest -from tagstudio.core.library.ignore import PATH_GLOB_FLAGS, Ignore, ignore_to_glob +from tagstudio.core.constants import IGNORE_NAME, TS_FOLDER_NAME +from tagstudio.core.library.alchemy.library import Library +from tagstudio.core.library.alchemy.models import Entry +from tagstudio.core.library.alchemy.registries.ignored_registry import IgnoredRegistry +from tagstudio.core.library.ignore import ( + Ignore, + IgnoreMatcher, + migrate_ext_list, +) +from tagstudio.core.library.scanners import _scan_with_internal_scanner, _scan_with_ripgrep +from tagstudio.core.utils.ripgrep_status import RipgrepStatus -def matches(patterns: list[str], path: str) -> bool: - return glob.compile(ignore_to_glob(patterns), flags=PATH_GLOB_FLAGS).match(path) +def is_ignored(patterns: list[str], path: str) -> bool: + return IgnoreMatcher(patterns).is_ignored(path) -def test_ignore_to_glob_does_not_crash_on_negated_root_anchored_pattern(): - """A pattern like "!/keep.txt" (negated + root-anchored) must not raise an error.""" +def test_negated_root_anchored_pattern(): + """A negated and root-anchored pattern (e.g. "!/keep.txt") only reincludes the root file.""" patterns = ["*.txt", "!/keep_this.txt"] - glob_patterns = ignore_to_glob(patterns) + assert is_ignored(patterns, "keep_this.txt") is False + assert is_ignored(patterns, "sub/keep_this.txt") is True + assert is_ignored(patterns, "other.txt") is True - assert matches(patterns, "keep_this.txt") is False - assert matches(patterns, "sub/keep_this.txt") is True - assert matches(patterns, "other.txt") is True - assert glob_patterns - -def test_ignore_to_glob_root_anchored_directory_matches_contents(): - """A root-anchored pattern for a directory (e.g. "/Downloads/") must exclude its contents.""" +def test_root_anchored_directory_ignores_contents(): + """A root-anchored directory pattern (e.g. "/Downloads/") must exclude its contents.""" patterns = ["/Downloads/"] - assert matches(patterns, "Downloads/file.txt") is True - assert matches(patterns, "sub/Downloads/file.txt") is False + assert is_ignored(patterns, "Downloads/file.txt") is True + assert is_ignored(patterns, "sub/Downloads/file.txt") is False -def test_ignore_to_glob_root_anchored_directory_without_trailing_slash(): - """A root-anchored pattern without a trailing slash still excludes contents.""" +def test_root_anchored_pattern_without_trailing_slash(): + """A root-anchored pattern without a trailing slash matches a file or a folder's contents.""" patterns = ["/Downloads"] - assert matches(patterns, "Downloads") is True - assert matches(patterns, "Downloads/file.txt") is True - assert matches(patterns, "sub/Downloads/file.txt") is False + assert is_ignored(patterns, "Downloads") is True + assert is_ignored(patterns, "Downloads/file.txt") is True + assert is_ignored(patterns, "sub/Downloads/file.txt") is False -def test_ignore_to_glob_non_rooted_directory_pattern(): - """A bare/non-rooted directory pattern matches at every depth.""" +def test_non_rooted_directory_pattern(): + """A non-rooted directory pattern matches folders at every depth, but never files.""" patterns = ["Dev/"] - assert matches(patterns, "Dev/file.txt") is True - assert matches(patterns, "sub/Dev/file.txt") is True - assert matches(patterns, "Dev") is False - assert matches(patterns, "SomeDevFile.txt") is False + assert is_ignored(patterns, "Dev/file.txt") is True + assert is_ignored(patterns, "sub/Dev/file.txt") is True + assert is_ignored(patterns, "Dev") is False + assert is_ignored(patterns, "SomeDevFile.txt") is False -def test_ignore_to_glob_leading_globstar_matches_zero_directories(): +def test_leading_globstar_matches_zero_directories(): """ "**/foo" must also match a root-level "foo", not just nested ones. NOTE: wcmatch's "**" only matches "one or more" directories while gitignore/ripgrep matches "zero or more", which is the target behavior. """ patterns = ["**/foo"] - assert matches(patterns, "foo") is True - assert matches(patterns, "a/foo") is True - assert matches(patterns, "a/b/foo") is True - assert matches(patterns, "foobar") is False + assert is_ignored(patterns, "foo") is True + assert is_ignored(patterns, "a/foo") is True + assert is_ignored(patterns, "a/b/foo") is True + assert is_ignored(patterns, "foobar") is False -def test_ignore_to_glob_middle_globstar_matches_zero_directories(): +def test_middle_globstar_matches_zero_directories(): """ "a/**/b" must also match "a/b".""" patterns = ["a/**/b"] - assert matches(patterns, "a/b") is True - assert matches(patterns, "a/x/b") is True - assert matches(patterns, "a/x/y/b") is True - assert matches(patterns, "a/bx") is False + assert is_ignored(patterns, "a/b") is True + assert is_ignored(patterns, "a/x/b") is True + assert is_ignored(patterns, "a/x/y/b") is True + assert is_ignored(patterns, "a/bx") is False - assert matches(patterns, "a/b/file.txt") is True - assert matches(patterns, "a/x/b/file.txt") is True - assert matches(patterns, "a/x/y/b/file.txt") is True - assert matches(patterns, "a/bx/file.txt") is False + assert is_ignored(patterns, "a/b/file.txt") is True + assert is_ignored(patterns, "a/x/b/file.txt") is True + assert is_ignored(patterns, "a/x/y/b/file.txt") is True + assert is_ignored(patterns, "a/bx/file.txt") is False -def test_ignore_to_glob_escaped_special_characters(): +def test_escaped_special_characters(): """A backslash before "#" or "!" must escape these characters.""" - assert matches(["\\#hashtag.jpg"], "#hashtag.jpg") is True - assert matches(["\\#hashtag.jpg"], "hashtag.jpg") is False - assert matches(["\\!wowee.jpg"], "!wowee.jpg") is True - assert matches(["\\!wowee.jpg"], "wowee.jpg") is False - - -def test_ignore_to_glob_output_has_no_duplicates(): - """Output must be deduplicated.""" - glob_patterns = ignore_to_glob(["*.jpg", "Photos/", "**/foo"]) - assert len(glob_patterns) == len(set(glob_patterns)) + assert is_ignored(["\\#hashtag.jpg"], "#hashtag.jpg") is True + assert is_ignored(["\\#hashtag.jpg"], "hashtag.jpg") is False + assert is_ignored(["\\!wowee.jpg"], "!wowee.jpg") is True + assert is_ignored(["\\!wowee.jpg"], "wowee.jpg") is False def test_single_asterisk_does_not_match_slash(): - """A single "*" must not match a single "/". - - fnmatch will still match "*" to a "/", when gitignore and wcmatch.glob will not. - """ + """A single "*" must not match a "/".""" patterns = ["Images/*.png"] - assert matches(patterns, "Images/mario.png") is True - assert matches(patterns, "Images/Mario/cat.png") is False - - -def test_negation_does_not_extend_to_deeper_subfolder(): - """A negation must not extend into a deeper subfolder its pattern doesn't match.""" - patterns = ["*.jpg", "!Photos/*.jpg", "Photos/Private/*.jpg"] - assert matches(patterns, "a.jpg") is True - assert matches(patterns, "Photos/a.jpg") is False - assert matches(patterns, "Photos/Private/a.jpg") is True + assert is_ignored(patterns, "Images/mario.png") is True + assert is_ignored(patterns, "Images/Mario/cat.png") is False def test_ignore_file_preserves_escaped_trailing_space(tmp_path: Path): @@ -127,3 +118,165 @@ def test_ignore_file_strips_crlf_line_ending(tmp_path: Path): ts_ignore = tmp_path / ".ts_ignore" ts_ignore.write_bytes(b"qux\r\n") assert Ignore._load_ignore_file(ts_ignore) == ["qux"] + + +def write_ts_ignore(library_dir: Path, content: str) -> None: + ts_ignore = library_dir / TS_FOLDER_NAME / IGNORE_NAME + ts_ignore.parent.mkdir(parents=True, exist_ok=True) + ts_ignore.write_text(content) + + +def touch(root: Path, paths: Iterable[str]) -> None: + for path in paths: + (root / path).parent.mkdir(parents=True, exist_ok=True) + (root / path).touch() + + +def test_library_patterns_override_built_in_patterns(tmp_path: Path): + """A library's own patterns take precedence over the built-in ones. + Mimics a global `.gitignore`'s behavior.""" + write_ts_ignore(tmp_path, "!.DS_Store\n") + Ignore.get_patterns(tmp_path) + assert Ignore.matcher.is_ignored(".DS_Store") is False + assert Ignore.matcher.is_ignored(".Trashes/a.png") is True + + +def test_ts_folder_cannot_be_reincluded(tmp_path: Path): + """Every .TagStudio folder at any depth must stay ignored, even if folders are reincluded.""" + write_ts_ignore(tmp_path, "*\n!*/\n!*.png\n") + Ignore.get_patterns(tmp_path) + assert Ignore.matcher.is_ignored(f"{TS_FOLDER_NAME}/x.png") is True + assert Ignore.matcher.is_ignored(f"sub/{TS_FOLDER_NAME}/x.png") is True + assert Ignore.matcher.is_ignored("sub/x.png") is False + + +def test_matcher_resets_for_a_library_without_a_ts_ignore(tmp_path: Path): + """Opening a library without a .ts_ignore must not keep the previous library's patterns.""" + with_ts_ignore = tmp_path / "with" + without_ts_ignore = tmp_path / "without" + write_ts_ignore(with_ts_ignore, "*.png\n") + without_ts_ignore.mkdir() + + Ignore.get_patterns(with_ts_ignore) + assert Ignore.matcher.is_ignored("a.png") is True + Ignore.get_patterns(without_ts_ignore) + assert Ignore.matcher.is_ignored("a.png") is False + + +def test_get_patterns_without_updating_state(tmp_path: Path): + """Getting a library's patterns with update_state=False must leave Ignore.matcher alone.""" + write_ts_ignore(tmp_path, "*.png\n") + assert "*.png" in Ignore.get_patterns(tmp_path, update_state=False) + assert Ignore.matcher.is_ignored("a.png") is False + + +def test_ignored_registry_respects_ignored_folders(library: Library, tmp_path: Path): + """The ignored registry must agree with the scanners about files inside ignored folders.""" + write_ts_ignore(tmp_path, "*\n!*.png\n") + Ignore.get_patterns(tmp_path) + paths = [Path("a.png"), Path("sub/b.png"), Path("sub/c.jpg")] + ids = library.add_entries([Entry(path=path, fields=[]) for path in paths]) + + registry = IgnoredRegistry(library) + list(registry.refresh_ignored_entries()) + ignored = {entry.path for entry in registry.ignored_entries if entry.id in ids} + assert ignored == {Path("sub/b.png"), Path("sub/c.jpg")} + + +def test_migrated_extension_include_list_keeps_nested_files(tmp_path: Path): + """An extension include list must keep matching files in subfolders after migrating.""" + write_ts_ignore(tmp_path, migrate_ext_list([".png"], is_exclude_list=False)) + matcher = IgnoreMatcher(Ignore.get_patterns(tmp_path, update_state=False)) + assert matcher.is_ignored("a.png") is False + assert matcher.is_ignored("sub/deep/a.png") is False + assert matcher.is_ignored("sub/deep/a.jpg") is True + + +# Expected results were generated with `git ls-files --others --exclude-standard` +GITIGNORE_TREE = { + "a.png", + "a.jpg", + ".hidden.png", + "sub/b.png", + "sub/b.jpg", + "sub/deep/c.png", + "sub/deep/c.txt", + "Photos/x.jpg", + "Photos/Private/y.jpg", + "build/out.o", + "src/build/gen.o", + "src/main.c", + "docs/build", + "#hash.txt", + "!bang.txt", + "keep/k.png", + "keep/k.txt", + "a/b", + "a/x/b", + "a/x/y/b/z.txt", +} +GITIGNORE_CASES: list[tuple[list[str], set[str]]] = [ + (["*", "!*.png"], GITIGNORE_TREE - {"a.png", ".hidden.png"}), + ( + ["*", "!*/", "!*.png"], + GITIGNORE_TREE - {"a.png", ".hidden.png", "sub/b.png", "sub/deep/c.png", "keep/k.png"}, + ), + (["*", "!*/"], GITIGNORE_TREE), + (["*", "!keep/", "!keep/**"], GITIGNORE_TREE - {"keep/k.png", "keep/k.txt"}), + ( + ["*.jpg", "!Photos/*.jpg", "Photos/Private/*.jpg"], + {"a.jpg", "sub/b.jpg", "Photos/Private/y.jpg"}, + ), + ( + ["*.png", "!a.png", "a.png"], + {"a.png", ".hidden.png", "sub/b.png", "sub/deep/c.png", "keep/k.png"}, + ), + (["*.png", "a.png", "!a.png"], {".hidden.png", "sub/b.png", "sub/deep/c.png", "keep/k.png"}), + (["Photos/", "!Photos/x.jpg"], {"Photos/x.jpg", "Photos/Private/y.jpg"}), + (["*.o", "!src/**/*.o"], {"build/out.o"}), + (["build/"], {"build/out.o", "src/build/gen.o"}), + (["/build/"], {"build/out.o"}), + (["build"], {"build/out.o", "src/build/gen.o", "docs/build"}), + (["sub/**"], {"sub/b.png", "sub/b.jpg", "sub/deep/c.png", "sub/deep/c.txt"}), + (["sub/*"], {"sub/b.png", "sub/b.jpg", "sub/deep/c.png", "sub/deep/c.txt"}), + (["sub/*/"], {"sub/deep/c.png", "sub/deep/c.txt"}), + (["**/deep"], {"sub/deep/c.png", "sub/deep/c.txt"}), + (["**/b"], {"a/b", "a/x/b", "a/x/y/b/z.txt"}), + (["a/**/b"], {"a/b", "a/x/b", "a/x/y/b/z.txt"}), + (["/a.png", "!/a.png"], set()), + (["\\#hash.txt", "\\!bang.txt"], {"#hash.txt", "!bang.txt"}), + ([".*"], {".hidden.png"}), + (["**"], GITIGNORE_TREE), + (["*/"], GITIGNORE_TREE - {"a.png", "a.jpg", ".hidden.png", "#hash.txt", "!bang.txt"}), + (["?.png"], {"a.png", "sub/b.png", "sub/deep/c.png", "keep/k.png"}), + (["[ab].*"], {"a.png", "a.jpg", "sub/b.png", "sub/b.jpg"}), + (["./a.png"], set()), +] + + +def scanned_files(paths: list[Path]) -> set[str]: + return {path.as_posix() for path in paths if path.parts[0] != TS_FOLDER_NAME} + + +@pytest.mark.parametrize(("patterns", "ignored"), GITIGNORE_CASES) +def test_matcher_follows_gitignore(patterns: list[str], ignored: set[str]): + """The ignore matcher must ignore exactly the same files as a .gitignore would.""" + matcher = IgnoreMatcher(patterns) + assert {path for path in GITIGNORE_TREE if matcher.is_ignored(path)} == ignored + + +@pytest.mark.parametrize(("patterns", "ignored"), GITIGNORE_CASES) +def test_internal_scanner_follows_gitignore(tmp_path: Path, patterns: list[str], ignored: set[str]): + """The internal scanner must skip exactly the same files as a .gitignore would.""" + touch(tmp_path, GITIGNORE_TREE) + scanned = scanned_files(list(_scan_with_internal_scanner(tmp_path, patterns))) + assert scanned == GITIGNORE_TREE - ignored + + +@pytest.mark.skipif(RipgrepStatus.which() is None, reason="ripgrep isn't installed") +@pytest.mark.parametrize(("patterns", "ignored"), GITIGNORE_CASES) +def test_ripgrep_scanner_follows_gitignore(tmp_path: Path, patterns: list[str], ignored: set[str]): + """The ripgrep scanner must skip exactly the same files as a .gitignore would.""" + touch(tmp_path, GITIGNORE_TREE) + scanned = scanned_files(list(_scan_with_ripgrep(tmp_path, patterns))) + assert scanned == GITIGNORE_TREE - ignored diff --git a/tests/core/library/test_migrations.py b/tests/core/library/test_migrations.py index 2568068425..4d4e5e11a6 100644 --- a/tests/core/library/test_migrations.py +++ b/tests/core/library/test_migrations.py @@ -7,7 +7,7 @@ import pytest -from tagstudio.core.constants import TS_FOLDER_NAME +from tagstudio.core.constants import IGNORE_NAME, TS_FOLDER_NAME from tagstudio.core.library.alchemy.constants import ( SQL_FILENAME, ) @@ -90,3 +90,30 @@ def test_migration_with_existing_field_template_tables(tmp_path: Path): } finally: library.close() + + +@pytest.mark.parametrize( + ("before", "after"), + [ + ("# Comment\n*\n!*.png\n*\n", "# Comment\n*\n!*/\n!*.png\n*\n"), + ("*\n!*/\n!*.png\n", "*\n!*/\n!*.png\n"), + ("*.jpg\n", "*.jpg\n"), + ], +) +def test_migration_to_500_reincludes_ts_ignore_folders(tmp_path: Path, before: str, after: str): + """A .ts_ignore that ignores everything with "*" must get "!*/" after the first occurrence + + If there's more than one occurrence... *why...* + """ + fixture = CWD.parents[2] / FIXTURES / EMPTY_LIBRARIES / "DB_VERSION_202" / TS_FOLDER_NAME + (tmp_path / TS_FOLDER_NAME).mkdir() + shutil.copy(fixture / SQL_FILENAME, tmp_path / TS_FOLDER_NAME / SQL_FILENAME) + ts_ignore = tmp_path / TS_FOLDER_NAME / IGNORE_NAME + ts_ignore.write_text(before) + + library = Library() + try: + assert library.open_library(library_dir=tmp_path).success + finally: + library.close() + assert ts_ignore.read_text() == after diff --git a/tests/core/library/test_sync.py b/tests/core/library/test_sync.py index 0fabcd7e94..448e487cbf 100644 --- a/tests/core/library/test_sync.py +++ b/tests/core/library/test_sync.py @@ -276,6 +276,46 @@ def test_sync_auto_relink_deleted_file(library: Library): assert Path("gone.txt") in {e.path for e in engine.unlinked_entries} +def add_tracked_files(library: Library, paths: list[Path], ts_ignore: str) -> list[int]: + """Create `paths` on disk with entries for them, then write the given .ts_ignore content.""" + library_dir = unwrap(library.library_dir) + for path in paths: + (library_dir / path).parent.mkdir(parents=True, exist_ok=True) + (library_dir / path).touch() + ts_ignore_path = library_dir / TS_FOLDER_NAME / IGNORE_NAME + ts_ignore_path.parent.mkdir(parents=True, exist_ok=True) + ts_ignore_path.write_text(ts_ignore) + return library.add_entries([Entry(path=path, fields=[]) for path in paths]) + + +@pytest.mark.parametrize("library", [TemporaryDirectory()], indirect=True) +def test_sync_files_skipped_by_ignore_rules_are_not_unlinked(library: Library): + """Existing files the scan skips because of ignore rules must never be unlinked or relinked.""" + library_dir = unwrap(library.library_dir) + engine = LibrarySyncEngine(library=library) + tracked = [Path("a.png"), Path("sub/b.png"), Path("sub/deep/c.png")] + tracked_ids = add_tracked_files(library, tracked, "*\n!*.png\n") + (library_dir / "c.png").touch() # A new file that could be mistaken for "sub/deep/c.png" + + list(engine.sync_dir(library_dir, force_internal_scanner=True)) + assert not {e.id for e in engine.unlinked_entries} & set(tracked_ids) + assert not {e.id for e in engine.relinked_entries} & set(tracked_ids) + assert [e.path for e in library.get_entries(tracked_ids)] == tracked + + +@pytest.mark.parametrize("library", [TemporaryDirectory()], indirect=True) +def test_sync_missing_file_in_ignored_folder_is_unlinked(library: Library): + """A file that's actually gone must still be unlinked, even if its folder is ignored.""" + library_dir = unwrap(library.library_dir) + engine = LibrarySyncEngine(library=library) + tracked_ids = add_tracked_files(library, [Path("sub/kept.png"), Path("sub/gone.png")], "sub/\n") + (library_dir / "sub" / "gone.png").unlink() + + list(engine.sync_dir(library_dir, force_internal_scanner=True)) + unlinked = {e.path for e in engine.unlinked_entries if e.id in tracked_ids} + assert unlinked == {Path("sub/gone.png")} + + @pytest.mark.parametrize("library", [TemporaryDirectory()], indirect=True) def test_sync_auto_relink_moved_and_renamed_file_modified(library: Library): """[Case #2] A moved, renamed, and modified file must not auto-relink."""