Honor dirconfig keep entries under alien indexing#2109
Merged
Conversation
Two things, the second found while looking at the first. Alien ignored `+' keep entries entirely: it walked the whole root and said nothing about it. The plumbing to restrict the walk already existed and hybrid used it - projectile-dir-files-alien and friends take a SUBDIRS argument, projectile--ext-command-line appends them as pathspecs with an fd --search-path special case - so alien just never passed them. Now it does. The other half is that appending pathspecs to a shell pipeline is broken. The svn, fossil and pijul recipes and the plain find fallback all end in a `tr' stage, so the paths landed on `tr' instead of the lister and the command exited non-zero: find . -type f | cut -c3- | tr '\n' '\0' src/ tests/ That is a live bug on master, not something alien introduced: hybrid with two or more keep entries hits it on any of those tools, which includes anyone without fd installed. Commands that can't take path arguments now run unrestricted and have their output filtered by path prefix instead, which is exact and cheap. Verified across native, hybrid, alien+git, alien+fd and both pipeline cases: all six now return the same file set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
alienignored+keep entries outright and said nothing about it, which is worst in exactly the monorepo case most likely to use them. The plumbing to restrict the walk already existed andhybridused it -projectile-dir-files-alienand friends take a SUBDIRS argument, andprojectile--ext-command-lineappends them as pathspecs with anfd --search-pathspecial case - so alien simply never passed them along.Looking at that turned up a live bug on master. Appending pathspecs to a shell pipeline doesn't restrict anything, it breaks the command: the svn, fossil and pijul recipes and the plain
findfallback all end in atrstage, so the paths land ontr.hybridwith two or more keep entries hits this on any of those tools, which includes anyone withoutfdinstalled - indexing fails with auser-errorrather than degrading. Commands that can't take path arguments now run unrestricted and get their output filtered by path prefix, which is exact and costs one pass.Verified across native, hybrid, alien+git, alien+fd and both pipeline cases; all six return the same file set for a project mixing
+keeps with a-ignore. The docs said+was hybrid/native only in three places, including the WARNING added a couple of hours ago - all updated, and the indexing comparison table now says Yes.