Skip to content

🧹 [code health] Centralize sys.path modifications #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ven0m0 merged 1 commit into from
Mar 8, 2026
Merged

🧹 [code health] Centralize sys.path modifications #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Scripts/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import sys
from pathlib import Path
scripts_dir = Path(__file__).parent
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 8, 2026

Choose a reason for hiding this comment

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

medium

For greater robustness, it's a good practice to resolve the path to an absolute path before adding it to sys.path. This ensures that you're always working with a canonical path, which prevents potential issues related to the current working directory, relative paths, or symlinks.

Suggested change
scripts_dir = Path(__file__).parent
scripts_dir = Path(__file__).parent.resolve()

if str(scripts_dir) not in sys.path:
sys.path.insert(0, str(scripts_dir))
Comment on lines +1 to +5
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

Scripts/init.py introduces a side-effectful sys.path mutation (inserting the Scripts dir at index 0). This can unexpectedly shadow/import the wrong modules (e.g., a third-party common module) and can also lead to duplicate module loads (common vs Scripts.common) if anything ever imports the package. Also, for the stated invocation python -m unittest discover Scripts ..., this init.py is typically not imported at all (tests are imported as top-level modules), so it likely doesn’t achieve the intended centralization. Prefer removing this sys.path injection (and possibly the file), or switching scripts/tests to package/relative imports (e.g., from .common ... / from Scripts.common ...) and running them via python -m when needed.

Suggested change
import sys
from pathlib import Path
scripts_dir = Path(__file__).parent
if str(scripts_dir) not in sys.path:
sys.path.insert(0, str(scripts_dir))
"""Package initializer for the Scripts module.
This file is intentionally kept free of side effects such as mutating
sys.path. Importers should rely on standard Python packaging and
relative imports instead of path manipulation.
"""

Copilot uses AI. Check for mistakes.
3 changes: 0 additions & 3 deletions Scripts/deduplicate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
from dataclasses import dataclass
from collections.abc import Iterable

# Add current directory to path to allow importing common if run from elsewhere
if str(Path(__file__).parent) not in sys.path:
sys.path.append(str(Path(__file__).parent))

from common import is_valid_domain, write_lines

Expand Down
2 changes: 0 additions & 2 deletions Scripts/move-pure-domains.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
from collections import defaultdict

# Import common utilities
if str(Path(__file__).parent) not in sys.path:
sys.path.append(str(Path(__file__).parent))

from common import is_valid_domain, read_lines, write_lines

Expand Down
7 changes: 2 additions & 5 deletions Scripts/test_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
from pathlib import Path
import hashlib

# Add current directory to path
if str(Path(__file__).parent) not in sys.path:
sys.path.append(str(Path(__file__).parent))

from common import sanitize_filename, is_valid_domain, is_adguard_rule

Expand Down Expand Up @@ -68,9 +65,9 @@ def test_sanitize_filename_without_name_special_urls(self):
# URL with port
url = "http://example.com:8080/list.txt"
filename = sanitize_filename(url)
self.assertTrue(filename.startswith("example-com:8080-"))
self.assertTrue(filename.startswith("example-com-8080-"))
expected_hash = hashlib.sha256(url.encode("utf-8")).hexdigest()[:12]
self.assertEqual(filename, f"example-com:8080-{expected_hash}.txt")
self.assertEqual(filename, f"example-com-8080-{expected_hash}.txt")

def test_is_valid_domain(self):
self.assertTrue(is_valid_domain("example.com"))
Expand Down
3 changes: 0 additions & 3 deletions Scripts/test_deduplicate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
import sys
from pathlib import Path
Comment on lines 2 to 3
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

After removing the sys.path bootstrap, sys and Path are no longer used here. Please remove the unused imports to avoid ruff/linters reporting F401 and to keep the test focused.

Suggested change
import sys
from pathlib import Path

Copilot uses AI. Check for mistakes.

# Add current directory to path to allow importing deduplicate
if str(Path(__file__).parent) not in sys.path:
sys.path.append(str(Path(__file__).parent))

from deduplicate import process_content

Expand Down
4 changes: 0 additions & 4 deletions Scripts/test_is_pure_domain_logic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
from pathlib import Path
import importlib

# Add Scripts to path
scripts_dir = Path(__file__).parent
if str(scripts_dir) not in sys.path:
sys.path.append(str(scripts_dir))

# Import ADGUARD_INDICATORS from common
from common import ADGUARD_INDICATORS
Expand Down
2 changes: 0 additions & 2 deletions Scripts/update-lists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
import aiofiles

# Import common utilities
if str(Path(__file__).parent) not in sys.path:
sys.path.append(str(Path(__file__).parent))

from common import sanitize_filename

Expand Down
Loading