Skip to content
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.

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

Use gitignore in streaming mode as well #28

Merged
merged 9 commits into from
Jan 20, 2025
Merged

Use gitignore in streaming mode as well #28

merged 9 commits into from
Jan 20, 2025

Conversation

mohsen1
Copy link
Collaborator

@mohsen1 mohsen1 commented Jan 20, 2025

Fixes #25

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR implements proper .gitignore handling in streaming mode by replacing WalkDir with WalkBuilder from the ignore crate.

  • Added WalkBuilder configuration in src/lib.rs to respect .gitignore rules consistently across both streaming and file output modes
  • Added comprehensive end-to-end tests in tests/test_gitignore_e2e.rs to verify ignore patterns work in both modes
  • Removed explicit gitignore check since WalkBuilder now handles it internally
  • Set follow_links to false to match Git's behavior for symlinks

2 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

src/lib.rs Outdated Show resolved Hide resolved
tests/test_gitignore_e2e.rs Outdated Show resolved Hide resolved
tests/test_gitignore_e2e.rs Outdated Show resolved Hide resolved
tests/test_gitignore_e2e.rs Show resolved Hide resolved
@mohsen1
Copy link
Collaborator Author

mohsen1 commented Jan 20, 2025

@greptileai review

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR extends the gitignore functionality improvements with additional test coverage and error handling refinements.

  • Added environment variables NO_COLOR and CI in tests/test_gitignore_e2e.rs to ensure consistent non-interactive test behavior
  • Improved error messages in file reading operations with detailed path information
  • Simplified binary file detection by removing user-defined extensions in streaming mode
  • Disabled git_global and git_exclude in WalkBuilder to only use local .gitignore files

3 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

let repo = setup_temp_repo();

// Create files without .gitignore
create_file(repo.path(), "binary.jpg", "ÿØÿÛ"); // JPEG magic bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

style: JPEG magic bytes are not properly encoded in the test file. Consider using actual binary data or hex encoding

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/parallel.rs Outdated Show resolved Hide resolved
@mohsen1 mohsen1 added this pull request to the merge queue Jan 20, 2025
Merged via the queue into main with commit c72e463 Jan 20, 2025
19 checks passed
@mohsen1 mohsen1 deleted the fix/issue-25 branch January 20, 2025 21:01
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.

gitignore is not respected in streaming mode
1 participant