- do not filter out same string from different modes

- use a consistent type for `seen` and pass in if we should deduplicate
- reuse a set accross searches

Author: KristofferC

stdlib/REPL/src/History/resumablefiltering.jl

@@ -253,24 +253,25 @@ end
 
 
 """
-    filterchunkrev!(out, candidates, spec; idx, maxtime, maxresults, seen) -> Int
+    filterchunkrev!(out, candidates, spec; idx, maxtime, maxresults, seen, deduplicate) -> 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 `seen` is provided (a Set{String}),
-only unique entries (by content) are added to avoid showing duplicate history items.
+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.
 """
 function filterchunkrev!(out::Vector{HistEntry}, candidates::DenseVector{HistEntry},
                          spec::FilterSpec, idx::Int = length(candidates);
                          maxtime::Float64 = Inf, maxresults::Int = length(candidates),
-                         seen::Union{Nothing,Set{String}} = nothing)
+                         seen::Set{Tuple{Symbol,String}} = Set{Tuple{Symbol,String}}(),
+                         deduplicate::Bool = false)
     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 !isnothing(seen) && entry.content ∈ seen
+            if deduplicate && (entry.mode, entry.content) ∈ seen
                 continue
             end
             if !isempty(spec.modes)
@@ -298,7 +299,7 @@ function filterchunkrev!(out::Vector{HistEntry}, candidates::DenseVector{HistEnt
                 end
             end
             matchfail && continue
-            !isnothing(seen) && push!(seen, entry.content)
+            deduplicate && push!(seen, (entry.mode, entry.content))
             pushfirst!(out, entry)
             length(out) == maxresults && break
         end

stdlib/REPL/src/History/search.jl

@@ -62,7 +62,8 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
     cands_temp = HistEntry[]
     # Filter state
     filter_idx = 0
-    filter_seen = nothing
+    filter_seen = Set{Tuple{Symbol,String}}()
+    isfiltering = false
     # Event loop
     while true
         event = @lock events if !isempty(events) take!(events) end
@@ -158,12 +159,12 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
             # 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)
-            filter_seen = isfiltering ? Set{String}() : nothing
+            empty!(filter_seen)
             filter_idx = filterchunkrev!(
                 state, cands_current;
                 maxtime = time() + 0.01,
                 maxresults = outsize[1],
-                seen = filter_seen)
+                seen = filter_seen, deduplicate = isfiltering)
             if filter_idx == 0
                 cands_cachestate = addcache!(
                     cands_cache, cands_cachestate, cands_cond => state.candidates)
@@ -195,7 +196,7 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
             filter_idx = filterchunkrev!(
                 state, cands_current, filter_idx;
                 maxtime = time() + 0.01,
-                seen = filter_seen)
+                seen = filter_seen, deduplicate = isfiltering)
             if filter_idx == 0
                 cands_cachestate = addcache!(
                     cands_cache, cands_cachestate, cands_cond => state.candidates)
@@ -213,10 +214,11 @@ end
 
 function filterchunkrev!(state::SelectorState, candidates::DenseVector{HistEntry}, idx::Int = length(candidates);
                          maxtime::Float64 = Inf, maxresults::Int = length(candidates),
-                         seen::Union{Nothing,Set{String}} = nothing)
+                         seen::Set{Tuple{Symbol,String}} = Set{Tuple{Symbol,String}}(),
+                         deduplicate::Bool = false)
     oldlen = length(state.candidates)
     idx = filterchunkrev!(state.candidates, candidates, state.filter, idx;
-                          maxtime = maxtime, maxresults = maxresults, seen = seen)
+                          maxtime = maxtime, maxresults = maxresults, seen = seen, deduplicate = deduplicate)
     newlen = length(state.candidates)
     newcands = view(state.candidates, (oldlen + 1):newlen)
     gfound = Int[]

stdlib/REPL/test/history.jl

@@ -346,7 +346,7 @@ end
             end
             @testset "Uniqueness" begin
                 empty!(results)
-                # Create entries with duplicate content
+                # Create entries with duplicate content in the same mode
                 dup_entries = [
                     HistEntry(:julia, now(UTC), "println(\"hello\")", 1),
                     HistEntry(:julia, now(UTC), "cos(2π)", 2),
@@ -359,8 +359,8 @@ end
                 # When filtering with seen Set, duplicates are removed
                 cset = ConditionSet("cos")
                 spec = FilterSpec(cset)
-                seen = Set{String}()
-                @test filterchunkrev!(results, dup_entries, spec; seen=seen) == 0
+                seen = Set{Tuple{Symbol,String}}()
+                @test filterchunkrev!(results, dup_entries, spec; seen=seen, deduplicate=true) == 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
@@ -372,6 +372,21 @@ end
                 @test filterchunkrev!(results, dup_entries, spec2) == 0
                 @test length(results) == 7  # All entries, including duplicates
                 @test results == dup_entries
+                # Test that same content in different modes is NOT deduplicated
+                empty!(results)
+                mode_entries = [
+                    HistEntry(:julia, now(UTC), "ls", 1),
+                    HistEntry(:shell, now(UTC), "ls", 2),
+                    HistEntry(:julia, now(UTC), "ls", 3),  # duplicate in :julia mode
+                    HistEntry(:shell, now(UTC), "pwd", 4),
+                ]
+                seen = Set{Tuple{Symbol,String}}()
+                cset3 = ConditionSet("ls")
+                spec3 = FilterSpec(cset3)
+                @test filterchunkrev!(results, mode_entries, spec3; seen=seen, deduplicate=true) == 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)
             end
         end
         @testset "Strictness comparison" begin