Skip to content

Correctly handle ignore pattern in multilevel nested histories#164

Closed
pfn-asm wants to merge 2 commits intoascmitc:masterfrom
pfn-djf:ignore_pattern_in_nested_histories
Closed

Correctly handle ignore pattern in multilevel nested histories#164
pfn-asm wants to merge 2 commits intoascmitc:masterfrom
pfn-djf:ignore_pattern_in_nested_histories

Conversation

@pfn-asm
Copy link
Collaborator

@pfn-asm pfn-asm commented May 16, 2025

This fix covers two main topics regarding ignore patterns in nested histories:

  • applying the ignore pattern in an existing nested history correctly (relative to the location of the child history)
  • writing an ignore pattern supplied to the create command into the respective child histories it actually affects (if it's not a global one)

@pfn-asm pfn-asm requested a review from Copilot May 16, 2025 13:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request fixes the handling of ignore patterns in multilevel nested histories and ensures that patterns are correctly propagated into child histories. Key changes include:

  • Enhancements in the logic to compute relevant ignore patterns in nested histories.
  • Introduction of new helper functions and updates to ignore spec calculations in generator and commands.
  • Updates to test fixtures to support the revised ignore pattern functionality.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_ignore.py Added a helper test function for asserting ignore pattern behavior.
tests/conftest.py Added fixtures to simulate complex nested file structures for testing.
ascmhl/utils.py Introduced a utility function to check if a path is absolute to a history.
ascmhl/history.py Added method to accumulate ignore patterns from nested histories.
ascmhl/generator.py Revised commit logic to use the new get_relevant_ignore_pattern helper.
ascmhl/commands.py Updated ignore spec handling to incorporate nested history ignores.
.github/workflows/build+test.yml Adjusted branch list to reflect the new feature branch.
Comments suppressed due to low confidence (1)

tests/test_ignore.py:85

  • The parameter name 'pattern' is reused as the loop variable in 'for pattern in pattern:', which shadows the outer variable. Consider renaming the loop variable to avoid confusion.
def assert_pattern_ignored_in_result(pattern: [str], result, negate=False):

return path


def check_path_is_absolute_to_history(base_path, relative_path) -> bool:
Copy link

Copilot AI May 16, 2025

Choose a reason for hiding this comment

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

Using 'startswith' to determine if one path is within another may lead to false positives (e.g., '/root/foo' vs '/root/foobar'). Consider using os.path.commonpath or a more robust method for path comparison.

Copilot uses AI. Check for mistakes.
@pfn-asm
Copy link
Collaborator Author

pfn-asm commented Jul 4, 2025

covered by pull request #166

@pfn-asm pfn-asm closed this Jul 4, 2025
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.

3 participants