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

Fix search when REPO_PATH contains a dot-dir #82

Merged
merged 3 commits into from
Oct 28, 2024
Merged

Conversation

DannyBen
Copy link
Owner

@DannyBen DannyBen commented Oct 27, 2024

cc #81

@DannyBen DannyBen changed the title Fix search when REPO_PATH had a dot-dir Fix search when REPO_PATH contains a dot-dir Oct 27, 2024
Comment on lines 14 to 17
find "$repo_path" -type d |
grep -v "$repo_path.*/\." |
grep --color=always "$text" |
sed "s#${repo_path}/#${prefix}#g"
Copy link

@pcrockett pcrockett Oct 27, 2024

Choose a reason for hiding this comment

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

Probably works fine for 99% of people, though I'm not super excited about the repo path being treated as a regex. Perhaps slightly more technically correct:

Suggested change
find "$repo_path" -type d |
grep -v "$repo_path.*/\." |
grep --color=always "$text" |
sed "s#${repo_path}/#${prefix}#g"
path_length=$((${#repo_path}+2))
find "$repo_path" -type d |
cut --characters "$path_length-" |
grep -v -F "/." |
grep --color=always "$text" |
sed "s#^#${prefix}#"

Copy link
Owner Author

Choose a reason for hiding this comment

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

I am not also not crazy about this excessive piping.
Perhaps we should search for dirs that contain main files instead of searching for dirs?

@pcrockett
Copy link

pcrockett commented Oct 27, 2024 via email

@DannyBen DannyBen merged commit 851cba9 into master Oct 28, 2024
4 checks passed
@DannyBen DannyBen deleted the fix/dot-dirs branch October 28, 2024 04:54
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