make seen obligatory and remove the deduplicate kwarg

Author: KristofferC

stdlib/REPL/src/History/resumablefiltering.jl

@@ -253,25 +253,23 @@ end
 
 
 """
-    filterchunkrev!(out, candidates, spec; idx, maxtime, maxresults, seen, deduplicate) -> Int
+    filterchunkrev!(out, candidates, spec, seen, idx; maxtime, maxresults) -> Int
 
 Incrementally filter `candidates[1:idx]` in reverse order.
 
 Pushes matches onto `out` until either `maxtime` is exceeded or `maxresults`
-collected, then returns the new resume index. When `deduplicate` is true,
-only unique entries (by mode and content) are added to avoid showing duplicate history items.
+collected, then returns the new resume index. Only unique entries (by mode and content)
+are added to avoid showing duplicate history items.
 """
 function filterchunkrev!(out::Vector{HistEntry}, candidates::DenseVector{HistEntry},
-                         spec::FilterSpec, idx::Int = length(candidates);
-                         maxtime::Float64 = Inf, maxresults::Int = length(candidates),
-                         seen::Set{Tuple{Symbol,String}} = Set{Tuple{Symbol,String}}(),
-                         deduplicate::Bool = false)
+                         spec::FilterSpec, seen::Set{Tuple{Symbol,String}}, idx::Int = length(candidates);
+                         maxtime::Float64 = Inf, maxresults::Int = length(candidates))
     batchsize = clamp(length(candidates) ÷ 512, 10, 1000)
     for batch in Iterators.partition(idx:-1:1, batchsize)
         time() > maxtime && break
         for outer idx in batch
             entry = candidates[idx]
-            if deduplicate && (entry.mode, entry.content) ∈ seen
+            if (entry.mode, entry.content) ∈ seen
                 continue
             end
             if !isempty(spec.modes)
@@ -299,7 +297,7 @@ function filterchunkrev!(out::Vector{HistEntry}, candidates::DenseVector{HistEnt
                 end
             end
             matchfail && continue
-            deduplicate && push!(seen, (entry.mode, entry.content))
+            push!(seen, (entry.mode, entry.content))
             pushfirst!(out, entry)
             length(out) == maxresults && break
         end

stdlib/REPL/src/History/search.jl

@@ -63,7 +63,6 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
     # Filter state
     filter_idx = 0
     filter_seen = Set{Tuple{Symbol,String}}()
-    isfiltering = false
     # Event loop
     while true
         event = @lock events if !isempty(events) take!(events) end
@@ -157,14 +156,19 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
             # Start filtering candidates
             # Only deduplicate when user has entered a search query. When browsing
             # with no filter (empty query), show all history including duplicates.
-            isfiltering = !isempty(filter_spec.exacts) || !isempty(filter_spec.negatives) ||
-                          !isempty(filter_spec.regexps) || !isempty(filter_spec.modes)
-            empty!(filter_seen)
-            filter_idx = filterchunkrev!(
-                state, cands_current;
-                maxtime = time() + 0.01,
-                maxresults = outsize[1],
-                seen = filter_seen, deduplicate = isfiltering)
+            if isempty(filter_spec.exacts) && isempty(filter_spec.negatives) &&
+               isempty(filter_spec.regexps) && isempty(filter_spec.modes)
+                # No filtering needed, just copy all candidates
+                append!(state.candidates, cands_current)
+                filter_idx = 0
+            else
+                # Filtering needed, deduplicate results
+                empty!(filter_seen)
+                filter_idx = filterchunkrev!(
+                    state, cands_current, filter_seen;
+                    maxtime = time() + 0.01,
+                    maxresults = outsize[1])
+            end
             if filter_idx == 0
                 cands_cachestate = addcache!(
                     cands_cache, cands_cachestate, cands_cond => state.candidates)
@@ -194,9 +198,8 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
                 state.area, state.query, state.filter, cands_temp,
                 state.scroll, state.selection, state.hover)
             filter_idx = filterchunkrev!(
-                state, cands_current, filter_idx;
-                maxtime = time() + 0.01,
-                seen = filter_seen, deduplicate = isfiltering)
+                state, cands_current, filter_seen, filter_idx;
+                maxtime = time() + 0.01)
             if filter_idx == 0
                 cands_cachestate = addcache!(
                     cands_cache, cands_cachestate, cands_cond => state.candidates)
@@ -212,13 +215,12 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
     end
 end
 
-function filterchunkrev!(state::SelectorState, candidates::DenseVector{HistEntry}, idx::Int = length(candidates);
-                         maxtime::Float64 = Inf, maxresults::Int = length(candidates),
-                         seen::Set{Tuple{Symbol,String}} = Set{Tuple{Symbol,String}}(),
-                         deduplicate::Bool = false)
+function filterchunkrev!(state::SelectorState, candidates::DenseVector{HistEntry},
+                         seen::Set{Tuple{Symbol,String}}, idx::Int = length(candidates);
+                         maxtime::Float64 = Inf, maxresults::Int = length(candidates))
     oldlen = length(state.candidates)
-    idx = filterchunkrev!(state.candidates, candidates, state.filter, idx;
-                          maxtime = maxtime, maxresults = maxresults, seen = seen, deduplicate = deduplicate)
+    idx = filterchunkrev!(state.candidates, candidates, state.filter, seen, idx;
+                          maxtime = maxtime, maxresults = maxresults)
     newlen = length(state.candidates)
     newcands = view(state.candidates, (oldlen + 1):newlen)
     gfound = Int[]

stdlib/REPL/test/history.jl

@@ -279,69 +279,80 @@ end
                 empty!(results)
                 cset = ConditionSet("hello")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen) == 0
                 @test results == [entries[1], entries[7]]
                 empty!(results)
                 cset2 = ConditionSet("world")
                 spec2 = FilterSpec(cset2)
-                @test filterchunkrev!(results, entries, spec2) == 0
+                empty!(seen)
+                @test filterchunkrev!(results, entries, spec2, seen) == 0
                 @test results == [entries[1], entries[7]]
                 empty!(results)
                 cset3 = ConditionSet("World")
                 spec3 = FilterSpec(cset3)
-                @test filterchunkrev!(results, entries, spec3) == 0
+                empty!(seen)
+                @test filterchunkrev!(results, entries, spec3, seen) == 0
                 @test results == [entries[7]]
             end
             @testset "Exact" begin
                 empty!(results)
                 cset = ConditionSet("=test")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec, maxresults = 2) == 5
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen; maxresults = 2) == 5
                 @test results == [entries[6], entries[9]]
                 empty!(results)
                 cset2 = ConditionSet("=test case")
                 spec2 = FilterSpec(cset2)
-                @test filterchunkrev!(results, entries, spec2) == 0
+                empty!(seen)
+                @test filterchunkrev!(results, entries, spec2, seen) == 0
                 @test results == [entries[3]]
             end
             @testset "Negative" begin
                 empty!(results)
                 cset = ConditionSet("!hello ; !test;! cos")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen) == 0
                 @test results == [entries[2], entries[7], entries[8]]
             end
             @testset "Initialism" begin
                 empty!(results)
                 cset = ConditionSet("`tc")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen) == 0
                 @test results == [entries[3]]
                 empty!(results)
                 cset2 = ConditionSet("`fb")
                 spec2 = FilterSpec(cset2)
-                @test filterchunkrev!(results, entries, spec2) == 0
+                empty!(seen)
+                @test filterchunkrev!(results, entries, spec2, seen) == 0
                 @test results == [entries[8]]
             end
             @testset "Regexp" begin
                 empty!(results)
                 cset = ConditionSet("/^c.s\\b")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen) == 0
                 @test results == [entries[4], entries[5]]
             end
             @testset "Mode" begin
                 empty!(results)
                 cset = ConditionSet(">shell")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen) == 0
                 @test results == [entries[7]]
             end
             @testset "Fuzzy" begin
                 empty!(results)
                 cset = ConditionSet("~cs")
                 spec = FilterSpec(cset)
-                @test filterchunkrev!(results, entries, spec) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, entries, spec, seen) == 0
                 @test results == entries[3:6]
             end
             @testset "Uniqueness" begin
@@ -360,16 +371,14 @@ end
                 cset = ConditionSet("cos")
                 spec = FilterSpec(cset)
                 seen = Set{Tuple{Symbol,String}}()
-                @test filterchunkrev!(results, dup_entries, spec; seen=seen, deduplicate=true) == 0
+                @test filterchunkrev!(results, dup_entries, spec, seen) == 0
                 # Should only get unique entries matching the filter
                 # Since we iterate in reverse (7->1), we keep the most recent occurrence of each unique content
                 @test length(results) == 1
                 @test results[1] == dup_entries[5]  # cos(2π) - most recent
-                # When browsing without seen Set, duplicates are kept
+                # When browsing without filtering, duplicates are kept
                 empty!(results)
-                cset2 = ConditionSet("")  # No filter
-                spec2 = FilterSpec(cset2)
-                @test filterchunkrev!(results, dup_entries, spec2) == 0
+                append!(results, dup_entries)
                 @test length(results) == 7  # All entries, including duplicates
                 @test results == dup_entries
                 # Test that same content in different modes is NOT deduplicated
@@ -380,10 +389,10 @@ end
                     HistEntry(:julia, now(UTC), "ls", 3),  # duplicate in :julia mode
                     HistEntry(:shell, now(UTC), "pwd", 4),
                 ]
-                seen = Set{Tuple{Symbol,String}}()
+                empty!(seen)
                 cset3 = ConditionSet("ls")
                 spec3 = FilterSpec(cset3)
-                @test filterchunkrev!(results, mode_entries, spec3; seen=seen, deduplicate=true) == 0
+                @test filterchunkrev!(results, mode_entries, spec3, seen) == 0
                 @test length(results) == 2  # "ls" from :julia and "ls" from :shell
                 @test results[1] == mode_entries[2]  # :shell ls
                 @test results[2] == mode_entries[3]  # :julia ls (most recent)