Skip to content

Commit 62a4e31

Browse files
author
KristofferC
committed
fix not looking at all conten in filterchunkrev! and also not remove duplicates when not filtering
1 parent 8f01412 commit 62a4e31

File tree

3 files changed

+36
-19
lines changed

3 files changed

+36
-19
lines changed

stdlib/REPL/src/History/resumablefiltering.jl

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,24 +253,26 @@ end
253253

254254

255255
"""
256-
filterchunkrev!(out, candidates, spec; idx, maxtime, maxresults) -> Int
256+
filterchunkrev!(out, candidates, spec; idx, maxtime, maxresults, seen) -> Int
257257
258258
Incrementally filter `candidates[1:idx]` in reverse order.
259259
260260
Pushes matches onto `out` until either `maxtime` is exceeded or `maxresults`
261-
collected, then returns the new resume index. Only unique entries (by content)
262-
are added to avoid showing duplicate history items.
261+
collected, then returns the new resume index. When `seen` is provided (a Set{String}),
262+
only unique entries (by content) are added to avoid showing duplicate history items.
263263
"""
264264
function filterchunkrev!(out::Vector{HistEntry}, candidates::DenseVector{HistEntry},
265265
spec::FilterSpec, idx::Int = length(candidates);
266-
maxtime::Float64 = Inf, maxresults::Int = length(candidates))
267-
seen = Set(e.content for e in out)
266+
maxtime::Float64 = Inf, maxresults::Int = length(candidates),
267+
seen::Union{Nothing,Set{String}} = nothing)
268268
batchsize = clamp(length(candidates) ÷ 512, 10, 1000)
269269
for batch in Iterators.partition(idx:-1:1, batchsize)
270270
time() > maxtime && break
271271
for outer idx in batch
272272
entry = candidates[idx]
273-
entry.content seen && continue
273+
if !isnothing(seen) && entry.content seen
274+
continue
275+
end
274276
if !isempty(spec.modes)
275277
entry.mode spec.modes || continue
276278
end
@@ -296,7 +298,7 @@ function filterchunkrev!(out::Vector{HistEntry}, candidates::DenseVector{HistEnt
296298
end
297299
end
298300
matchfail && continue
299-
push!(seen, entry.content)
301+
!isnothing(seen) && push!(seen, entry.content)
300302
pushfirst!(out, entry)
301303
length(out) == maxresults && break
302304
end

stdlib/REPL/src/History/search.jl

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
6262
cands_temp = HistEntry[]
6363
# Filter state
6464
filter_idx = 0
65+
filter_seen = nothing
6566
# Event loop
6667
while true
6768
event = @lock events if !isempty(events) take!(events) end
@@ -153,10 +154,16 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
153154
end
154155
end
155156
# Start filtering candidates
157+
# Only deduplicate when user has entered a search query. When browsing
158+
# with no filter (empty query), show all history including duplicates.
159+
isfiltering = !isempty(filter_spec.exacts) || !isempty(filter_spec.negatives) ||
160+
!isempty(filter_spec.regexps) || !isempty(filter_spec.modes)
161+
filter_seen = isfiltering ? Set{String}() : nothing
156162
filter_idx = filterchunkrev!(
157163
state, cands_current;
158164
maxtime = time() + 0.01,
159-
maxresults = outsize[1])
165+
maxresults = outsize[1],
166+
seen = filter_seen)
160167
if filter_idx == 0
161168
cands_cachestate = addcache!(
162169
cands_cache, cands_cachestate, cands_cond => state.candidates)
@@ -187,7 +194,8 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
187194
state.scroll, state.selection, state.hover)
188195
filter_idx = filterchunkrev!(
189196
state, cands_current, filter_idx;
190-
maxtime = time() + 0.01)
197+
maxtime = time() + 0.01,
198+
seen = filter_seen)
191199
if filter_idx == 0
192200
cands_cachestate = addcache!(
193201
cands_cache, cands_cachestate, cands_cond => state.candidates)
@@ -204,10 +212,11 @@ function run_display!((; term, pstate), events::Channel{Symbol}, hist::Vector{Hi
204212
end
205213

206214
function filterchunkrev!(state::SelectorState, candidates::DenseVector{HistEntry}, idx::Int = length(candidates);
207-
maxtime::Float64 = Inf, maxresults::Int = length(candidates))
215+
maxtime::Float64 = Inf, maxresults::Int = length(candidates),
216+
seen::Union{Nothing,Set{String}} = nothing)
208217
oldlen = length(state.candidates)
209218
idx = filterchunkrev!(state.candidates, candidates, state.filter, idx;
210-
maxtime = maxtime, maxresults = maxresults)
219+
maxtime = maxtime, maxresults = maxresults, seen = seen)
211220
newlen = length(state.candidates)
212221
newcands = view(state.candidates, (oldlen + 1):newlen)
213222
gfound = Int[]

stdlib/REPL/test/history.jl

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -356,16 +356,22 @@ end
356356
HistEntry(:julia, now(UTC), "println(\"hello\")", 6), # duplicate
357357
HistEntry(:julia, now(UTC), "tan(π/4)", 7),
358358
]
359-
cset = ConditionSet("") # Match all
359+
# When filtering with seen Set, duplicates are removed
360+
cset = ConditionSet("cos")
360361
spec = FilterSpec(cset)
361-
@test filterchunkrev!(results, dup_entries, spec) == 0
362-
# Should only get unique entries
362+
seen = Set{String}()
363+
@test filterchunkrev!(results, dup_entries, spec; seen=seen) == 0
364+
# Should only get unique entries matching the filter
363365
# Since we iterate in reverse (7->1), we keep the most recent occurrence of each unique content
364-
@test length(results) == 4
365-
@test results[1] == dup_entries[4] # sin(π)
366-
@test results[2] == dup_entries[5] # cos(2π) - most recent
367-
@test results[3] == dup_entries[6] # println("hello") - most recent
368-
@test results[4] == dup_entries[7] # tan(π/4)
366+
@test length(results) == 1
367+
@test results[1] == dup_entries[5] # cos(2π) - most recent
368+
# When browsing without seen Set, duplicates are kept
369+
empty!(results)
370+
cset2 = ConditionSet("") # No filter
371+
spec2 = FilterSpec(cset2)
372+
@test filterchunkrev!(results, dup_entries, spec2) == 0
373+
@test length(results) == 7 # All entries, including duplicates
374+
@test results == dup_entries
369375
end
370376
end
371377
@testset "Strictness comparison" begin

0 commit comments

Comments
 (0)