Support for multi-pattern search with -e/--pattern - #12
Conversation
dannote
left a comment
There was a problem hiding this comment.
Thanks for all the PRs, Victor — lots of good ideas here. I found a few issues with this one that need fixing before it can go in.
| {head, segments} = segment_argv(args) | ||
|
|
||
| {global_opts, head_positional, _} = | ||
| OptionParser.parse(head, strict: @global_switches ++ SelectorOptions.switches()) |
There was a problem hiding this comment.
Selector flags before the first -e end up in global_opts and are silently ignored. --inside X -e P file runs P unfiltered. Since the rule is that filters bind to the preceding -e, I think this should error. Could you add validation and a test for it?
| selector = SelectorOptions.scoped_pattern(pattern, filter_opts, &validate_pattern!/1) | ||
|
|
||
| {[%{pattern: pattern, selector: selector} | compiled], global_opts ++ seg_globals, | ||
| positional ++ paths} |
There was a problem hiding this comment.
This reverses path order. One segment with [a.ex, b.ex] becomes [b.ex, a.ex] after the final reverse, which matters with --limit. Could you preserve argv order and add a two-file test?
|
|
||
| defp build_named_patterns!(compiled) do | ||
| Enum.reduce(compiled, [], fn %{pattern: str, selector: selector}, acc -> | ||
| key = String.to_atom(str) |
There was a problem hiding this comment.
We shouldn't create atoms from CLI input. This leaks atoms in long-running VMs, and a valid pattern over 255 bytes raises SystemLimitError here. Could you use bounded internal IDs and map them back to the original strings?
Let `mix ex_ast.search` run several patterns in one invocation via a repeatable `-e`/`--pattern` flag, reading and parsing each file once for the whole batch through `ExAST.search_many/3` — avoiding BEAM startup and per-file re-parsing per pattern. - No `-e`: unchanged single-pattern path via `ExAST.search/3`. - One or more `-e`: multi-pattern batch; positional pattern is rejected. - Selector filters (`--inside`, `--parent`, etc.) bind to the most recent preceding `-e`, grep-style. argv is segmented manually since OptionParser loses interleaving order, then each segment compiles its own Selector via the new `SelectorOptions.scoped_pattern/3`. - Global flags (`--count`, `--json`, `--expand-imports`, `--limit`, `--allow-broad`, paths) apply to the whole batch. - Output tags each match by its pattern string; `--count` gives a per-pattern tally plus total; `--json` carries the `:pattern` field. - Duplicate `-e` patterns raise; `--count-by-file` is rejected with `-e`. Per-pattern path scoping is not expressible in one call (shared path list); noted in the @moduledoc.
442d01b to
4a0c4a4
Compare
With this
mix ex_ast.searchcan run many patterns in one invocation — one BEAM boot, one parse per file for the whole batch — via a repeatable-e/--patternflag, mirroringgrep -e/rg -e.The library already supported batching (
ExAST.search_many/3, which parses each file once and tags every match with a:pattern); this fills the CLI gap. Analyzers and linters that run dozens of checks over the same tree stop paying startup + re-parse per pattern.The single-pattern path (
ExAST.search/3) is untouched — the task branches early on whether any-ewas given.'PATTERN' path/-e A -e B path/-e A --inside X -e B-e(grep model)'PATTERN' -e A-ePer-pattern selector filters
Selector filters (
--inside,--not-inside,--parent,--contains, …) are per-pattern: each-ecompiles its ownSelector, and filters don't bleed across patterns.OptionParser's:keeploses interleaving order, soargvis segmented manually — each-eplus the filters that follow it up to the next-eform one segment. Global flags (--count,--json,--expand-imports,--limit,--allow-broad, paths) apply to the whole batch.Before
-ewasn't a flag — batching was only reachable from the library:After
--countprints a per-pattern tally plus a total:--jsoncarries thepatternfield on each match (already present onsearch_manyresults).Patterns are tagged by their raw string; duplicate
-estrings raise rather than silently merging.--count-by-fileis rejected with-e(per-pattern file totals aren't meaningful across a shared batch) rather than silently ignored.