Skip to content

add_lang_ext_support_to_diff#1858

Merged
Saga4 merged 1 commit intomainfrom
fix/add_lang_ext_support_to_diff
Mar 18, 2026
Merged

add_lang_ext_support_to_diff#1858
Saga4 merged 1 commit intomainfrom
fix/add_lang_ext_support_to_diff

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Mar 17, 2026

No description provided.

@claude
Copy link
Contributor

claude bot commented Mar 17, 2026

Claude finished @Saga4's task in 1m 55s —— View job


PR Review Summary

  • Triage PR scope
  • Run lint and type checks
  • Check for stale review threads
  • Review code changes
  • Check for duplicate logic
  • Analyze test coverage
  • Post summary

Prek Checks

All checks pass — ruff format and ruff lint both clean on changed files.

Code Review

The change is simple and correct. Instead of hardcoding .py in the filter:

# Before
if file_path.suffix != ".py":
    continue

# After
supported_extensions = set(get_supported_extensions())
...
if file_path.suffix not in supported_extensions:
    continue

This makes get_git_diff aware of all registered languages (Python, JavaScript/TypeScript, etc.) via the existing registry. The supported_extensions set is computed once before the loop, which is good. The extensions stored in _EXTENSION_REGISTRY use leading dots (.py, .js, .ts, etc.), which matches Path.suffix output — no format mismatch.

Minor observation: The extensions in the registry are stored as lowercase (ext.lower() at registry.py:118), while Path.suffix on Linux is case-sensitive. Files named example.PY would be skipped. This is a pre-existing concern and not introduced by this PR.

Duplicate Detection

No duplicates detected. The two other .py-extension checks found elsewhere (edit_generated_tests.py:154, github_workflow.py:591) serve different purposes (Python-specific logic, not language filtering) and should not be changed.

Test Coverage

All 12 existing test_git_utils.py tests pass. The existing tests use .py diff fixtures, which continue to work since Python is still a supported extension via the registry. No tests were added for non-Python files (e.g., .js, .ts) being included/excluded by the new dynamic filter — this would be a nice addition but is non-blocking for a simple, low-risk change like this.


Last updated: 2026-03-17

@Saga4 Saga4 requested a review from aseembits93 March 17, 2026 23:41
@aseembits93
Copy link
Contributor

i think @mashraf-222 already opened it -> #1855

@Saga4 Saga4 merged commit 32decd7 into main Mar 18, 2026
27 checks passed
@Saga4 Saga4 deleted the fix/add_lang_ext_support_to_diff branch March 18, 2026 13:15
@Saga4 Saga4 restored the fix/add_lang_ext_support_to_diff branch March 18, 2026 13:33
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.

2 participants