From 7b08a4604789251459131319fa595a7413790f18 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Thu, 10 Aug 2017 09:20:59 +0200 Subject: [PATCH 01/11] re-enable terminal's beep and use... something else! In the REPL, when an action cannot be done, the user can be notified by animating the prompt - no need to use built-in terminal's beep ('\a'), which is anyway often disabled. --- base/repl/LineEdit.jl | 67 ++++++++++++++++++++++++++++++++++-------- base/repl/REPL.jl | 8 ++--- base/repl/Terminals.jl | 2 +- 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index a2fa7e245e796..921372db823e1 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -9,6 +9,9 @@ import ..Terminals: raw!, width, height, cmove, getX, import Base: ensureroom, peek, show, AnyDict, position +const REFRESH_LOCK = Threads.SpinLock() +const BEEP_LOCK = Threads.SpinLock() + abstract type TextInterface end abstract type ModeState end @@ -113,9 +116,46 @@ complete_line(c::EmptyCompletionProvider, s) = [], true, true terminal(s::IO) = s terminal(s::PromptState) = s.terminal +const beeping = Threads.Atomic{Float64}(0.0) + +function beep(s::PromptState, duration=0.2, blink=0.2, maxduration=1.0; + colors=[Base.text_colors[:light_black]], + use_current::Bool=true, + underline::Bool=false) + + Threads.atomic_add!(beeping, min(duration, maxduration-beeping[])) + @async begin + trylock(BEEP_LOCK) || return + orig_prefix = s.p.prompt_prefix + use_current && push!(colors, orig_prefix) + i = 0 + while beeping[] > 0.0 + prefix = colors[mod1(i+=1, end)] * Base.text_colors[:underline]^underline + s.p.prompt_prefix = prefix + refresh_multi_line(s) + sleep(blink) + Threads.atomic_sub!(beeping, blink) + end + s.p.prompt_prefix = orig_prefix + refresh_multi_line(s) + beeping[] = 0.0 + unlock(BEEP_LOCK) + end +end + +function cancel_beep(s::PromptState) + beeping[] = 0.0 + # wait till beeping finishes + while !trylock(BEEP_LOCK) sleep(.05) end + unlock(BEEP_LOCK) +end + +beep(s) = nothing +cancel_beep(s) = nothing + for f in [:terminal, :on_enter, :add_history, :buffer, :(Base.isempty), :replace_line, :refresh_multi_line, :input_string, :update_display_buffer, - :empty_undo, :push_undo, :pop_undo] + :empty_undo, :push_undo, :pop_undo, :cancel_beep, :beep] @eval ($f)(s::MIState, args...) = $(f)(s.mode_state[s.current_mode], args...) end @@ -180,7 +220,7 @@ end function complete_line(s::PromptState, repeats) completions, partial, should_complete = complete_line(s.p.complete, s) if isempty(completions) - beep(terminal(s)) + beep(s) elseif !should_complete # should_complete is false for cases where we only want to show # a list of possible completions but not complete, e.g. foo(\t @@ -231,6 +271,7 @@ refresh_multi_line(s::ModeState) = refresh_multi_line(terminal(s), s) refresh_multi_line(termbuf::TerminalBuffer, s::ModeState) = refresh_multi_line(termbuf, terminal(s), s) refresh_multi_line(termbuf::TerminalBuffer, term, s::ModeState) = (@assert term == terminal(s); refresh_multi_line(termbuf,s)) function refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, buf, state::InputAreaState, prompt = ""; indent = 0) + lock(REFRESH_LOCK) _clear_input_area(termbuf, state) cols = width(terminal) @@ -294,7 +335,7 @@ function refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, buf #columns are 1 based cmove_col(termbuf, curs_pos + 1) - + unlock(REFRESH_LOCK) # Updated cur_row,curs_row return InputAreaState(cur_row, curs_row) end @@ -554,7 +595,7 @@ function edit_backspace(s::PromptState, align::Bool=false, adjust=align) refresh_line(s) else pop_undo(s) - beep(terminal(s)) + beep(s) end end @@ -603,7 +644,7 @@ function edit_delete(s) refresh_line(s) else pop_undo(s) - beep(terminal(s)) + beep(s) end :edit_delete end @@ -836,7 +877,7 @@ function history_prev(s, hist) move_input_start(s) refresh_line(s) else - beep(terminal(s)) + beep(s) end end function history_next(s, hist) @@ -846,7 +887,7 @@ function history_next(s, hist) move_input_end(s) refresh_line(s) else - beep(terminal(s)) + beep(s) end end @@ -1228,12 +1269,12 @@ end terminal(s::SearchState) = s.terminal function update_display_buffer(s::SearchState, data) - history_search(data.histprompt.hp, data.query_buffer, data.response_buffer, data.backward, false) || beep(terminal(s)) + history_search(data.histprompt.hp, data.query_buffer, data.response_buffer, data.backward, false) || beep(s) refresh_line(s) end function history_next_result(s::MIState, data::SearchState) - history_search(data.histprompt.hp, data.query_buffer, data.response_buffer, data.backward, true) || beep(terminal(s)) + history_search(data.histprompt.hp, data.query_buffer, data.response_buffer, data.backward, true) || beep(s) refresh_line(data) end @@ -1435,15 +1476,15 @@ function setup_search_keymap(hp) # Backspace/^H '\b' => (s,data,c)->(edit_backspace(data.query_buffer) ? - update_display_buffer(s, data) : beep(terminal(s))), + update_display_buffer(s, data) : beep(s)), 127 => KeyAlias('\b'), # Meta Backspace "\e\b" => (s,data,c)->(edit_delete_prev_word(data.query_buffer) ? - update_display_buffer(s, data) : beep(terminal(s))), + update_display_buffer(s, data) : beep(s)), "\e\x7f" => "\e\b", # Word erase to whitespace "^W" => (s,data,c)->(edit_werase(data.query_buffer) ? - update_display_buffer(s, data) : beep(terminal(s))), + update_display_buffer(s, data) : beep(s)), # ^C and ^D "^C" => (s,data,c)->(edit_clear(data.query_buffer); edit_clear(data.response_buffer); @@ -1544,6 +1585,7 @@ function move_line_end(buf::IOBuffer) end function commit_line(s) + cancel_beep(s) move_input_end(s) refresh_line(s) println(terminal(s)) @@ -1690,6 +1732,7 @@ AnyDict( try # raise the debugger if present ccall(:jl_raise_debugger, Int, ()) end + cancel_beep(s) move_input_end(s) refresh_line(s) print(terminal(s), "^C\n\n") diff --git a/base/repl/REPL.jl b/base/repl/REPL.jl index c8fc5a3fb42ba..f998511f1ddb0 100644 --- a/base/repl/REPL.jl +++ b/base/repl/REPL.jl @@ -477,14 +477,14 @@ function history_prev(s::LineEdit.MIState, hist::REPLHistoryProvider, elseif m === :skip history_prev(s, hist, num+1, save_idx) else - Terminals.beep(LineEdit.terminal(s)) + Terminals.beep(s) end end function history_next(s::LineEdit.MIState, hist::REPLHistoryProvider, num::Int=1, save_idx::Int = hist.cur_idx) if num == 0 - Terminals.beep(LineEdit.terminal(s)) + Terminals.beep(s) return end num < 0 && return history_prev(s, hist, -num, save_idx) @@ -502,7 +502,7 @@ function history_next(s::LineEdit.MIState, hist::REPLHistoryProvider, elseif m === :skip history_next(s, hist, num+1, save_idx) else - Terminals.beep(LineEdit.terminal(s)) + Terminals.beep(s) end end @@ -546,7 +546,7 @@ function history_move_prefix(s::LineEdit.PrefixSearchState, end end end - Terminals.beep(LineEdit.terminal(s)) + Terminals.beep(s) end history_next_prefix(s::LineEdit.PrefixSearchState, hist::REPLHistoryProvider, prefix::AbstractString) = history_move_prefix(s, hist, prefix, false) diff --git a/base/repl/Terminals.jl b/base/repl/Terminals.jl index ad7382009d832..1941043a5e65b 100644 --- a/base/repl/Terminals.jl +++ b/base/repl/Terminals.jl @@ -146,7 +146,7 @@ end @eval clear(t::UnixTerminal) = write(t.out_stream, $"$(CSI)H$(CSI)2J") @eval clear_line(t::UnixTerminal) = write(t.out_stream, $"\r$(CSI)0K") -#beep(t::UnixTerminal) = write(t.err_stream,"\x7") +beep(t::UnixTerminal) = write(t.err_stream,"\x7") Base.displaysize(t::UnixTerminal) = displaysize(t.out_stream) From 081ed9e3660309e96857319ce8147e17f363a599 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Tue, 15 Aug 2017 12:01:10 +0200 Subject: [PATCH 02/11] do not use Thread.Atomic variables (not supported everywhere) It's probably not crucial for this functionality to have atomicity. --- base/repl/LineEdit.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 921372db823e1..c6e1755a951bb 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -116,14 +116,14 @@ complete_line(c::EmptyCompletionProvider, s) = [], true, true terminal(s::IO) = s terminal(s::PromptState) = s.terminal -const beeping = Threads.Atomic{Float64}(0.0) +const beeping = Ref(0.0) function beep(s::PromptState, duration=0.2, blink=0.2, maxduration=1.0; colors=[Base.text_colors[:light_black]], use_current::Bool=true, underline::Bool=false) - Threads.atomic_add!(beeping, min(duration, maxduration-beeping[])) + beeping[] = min(beeping[]+duration, maxduration) @async begin trylock(BEEP_LOCK) || return orig_prefix = s.p.prompt_prefix @@ -134,7 +134,7 @@ function beep(s::PromptState, duration=0.2, blink=0.2, maxduration=1.0; s.p.prompt_prefix = prefix refresh_multi_line(s) sleep(blink) - Threads.atomic_sub!(beeping, blink) + beeping[] -= blink end s.p.prompt_prefix = orig_prefix refresh_multi_line(s) From e42b591fbf35d7ab8a024566c5ccdc14aae875cc Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Thu, 24 Aug 2017 18:35:57 +0200 Subject: [PATCH 03/11] use trylock in refresh_multi_line --- base/repl/LineEdit.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index c6e1755a951bb..baef0f021f3c9 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -271,7 +271,7 @@ refresh_multi_line(s::ModeState) = refresh_multi_line(terminal(s), s) refresh_multi_line(termbuf::TerminalBuffer, s::ModeState) = refresh_multi_line(termbuf, terminal(s), s) refresh_multi_line(termbuf::TerminalBuffer, term, s::ModeState) = (@assert term == terminal(s); refresh_multi_line(termbuf,s)) function refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, buf, state::InputAreaState, prompt = ""; indent = 0) - lock(REFRESH_LOCK) + while !trylock(REFRESH_LOCK) sleep(.01) end _clear_input_area(termbuf, state) cols = width(terminal) From aa1ef9fc95e3dd8d75987ed7ecda0a274284b4aa Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Wed, 6 Sep 2017 18:11:51 +0200 Subject: [PATCH 04/11] use isinteractive() in beep to try to have tests pass on CI --- base/repl/LineEdit.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index baef0f021f3c9..8a1ef9233b6b0 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -122,7 +122,7 @@ function beep(s::PromptState, duration=0.2, blink=0.2, maxduration=1.0; colors=[Base.text_colors[:light_black]], use_current::Bool=true, underline::Bool=false) - + isinteractive() || return beeping[] = min(beeping[]+duration, maxduration) @async begin trylock(BEEP_LOCK) || return @@ -704,7 +704,7 @@ end function edit_yank(s::MIState) if isempty(s.kill_ring) - beep(terminal(s)) + beep(s) return :ignore end setmark(s) # necessary for edit_yank_pop @@ -717,7 +717,7 @@ end function edit_yank_pop(s::MIState, require_previous_yank=true) if require_previous_yank && !(s.last_action in [:edit_yank, :edit_yank_pop]) || isempty(s.kill_ring) - beep(terminal(s)) + beep(s) :ignore else push_undo(s) @@ -1710,7 +1710,7 @@ AnyDict( # Meta Enter "\e\r" => (s,o...)->edit_insert_newline(s), "\e\n" => "\e\r", - "^_" => (s,o...)->(pop_undo(s) ? refresh_line(s) : beep(terminal(s))), + "^_" => (s,o...)->(pop_undo(s) ? refresh_line(s) : beep(s)), # Simply insert it into the buffer by default "*" => (s,data,c)->(edit_insert(s, c)), "^U" => (s,o...)->edit_clear(s), From c8dd8dce01c9b4ca90fc105b3490ba5d0784706f Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Sun, 3 Sep 2017 15:46:02 +0200 Subject: [PATCH 05/11] REPL: more deleted words pushed into the kill ring The two ways of deleting the previous word, deleting the next word and clearing the space now hook into the kill ring. Implements item 2 of #8447. --- base/repl/LineEdit.jl | 48 +++++++++++++++++++++++++++---------------- test/lineedit.jl | 33 ++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 95a05868bce22..ebf27b7a7bad9 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -621,45 +621,54 @@ function edit_werase(buf::IOBuffer) pos1 = position(buf) char_move_word_left(buf, isspace) pos0 = position(buf) - pos0 < pos1 || return false edit_splice!(buf, pos0 => pos1) - true end function edit_werase(s::MIState) push_undo(s) - edit_werase(buffer(s)) ? refresh_line(s) : pop_undo(s) - :edit_werase + if push_kill!(s, edit_werase(buffer(s)), rev=true) + refresh_line(s) + :edit_werase + else + pop_undo(s) + :ignore + end end function edit_delete_prev_word(buf::IOBuffer) pos1 = position(buf) char_move_word_left(buf) pos0 = position(buf) - pos0 < pos1 || return false edit_splice!(buf, pos0 => pos1) - true end function edit_delete_prev_word(s::MIState) push_undo(s) - edit_delete_prev_word(buffer(s)) ? refresh_line(s) : pop_undo(s) - :edit_delete_prev_word + if push_kill!(s, edit_delete_prev_word(buffer(s)), rev=true) + refresh_line(s) + :edit_delete_prev_word + else + pop_undo(s) + :ignore + end end function edit_delete_next_word(buf::IOBuffer) pos0 = position(buf) char_move_word_right(buf) pos1 = position(buf) - pos0 < pos1 || return false edit_splice!(buf, pos0 => pos1) - true end function edit_delete_next_word(s) push_undo(s) - edit_delete_next_word(buffer(s)) ? refresh_line(s) : pop_undo(s) - :edit_delete_next_word + if push_kill!(s, edit_delete_next_word(buffer(s))) + refresh_line(s) + :edit_delete_next_word + else + pop_undo(s) + :ignore + end end function edit_yank(s::MIState) @@ -687,10 +696,12 @@ function edit_yank_pop(s::MIState, require_previous_yank=true) end end -function push_kill!(s::MIState, killed::String, concat=false) +function push_kill!(s::MIState, killed::String, concat = s.key_repeats > 0; rev=false) isempty(killed) && return false - if concat - s.kill_ring[end] *= killed + if concat && !isempty(s.kill_ring) + s.kill_ring[end] = rev ? + killed * s.kill_ring[end] : # keep expected order for backward deletion + s.kill_ring[end] * killed else push!(s.kill_ring, killed) length(s.kill_ring) > KILL_RING_MAX[] && shift!(s.kill_ring) @@ -708,7 +719,7 @@ function edit_kill_line(s::MIState) killbuf = killbuf[1:end-1] char_move_left(buf) end - push_kill!(s, killbuf, s.key_repeats > 0) || return :ignore + push_kill!(s, killbuf) || return :ignore edit_splice!(buf, pos => position(buf)) refresh_line(s) :edit_kill_line @@ -716,7 +727,7 @@ end function edit_copy_region(s::MIState) buf = buffer(s) - push_kill!(s, content(buf, region(buf))) || return :ignore + push_kill!(s, content(buf, region(buf)), false) || return :ignore if REGION_ANIMATION_DURATION[] > 0.0 edit_exchange_point_and_mark(s) sleep(REGION_ANIMATION_DURATION[]) @@ -727,7 +738,7 @@ end function edit_kill_region(s::MIState) push_undo(s) - if push_kill!(s, edit_splice!(s)) + if push_kill!(s, edit_splice!(s), false) refresh_line(s) :edit_kill_region else @@ -807,6 +818,7 @@ edit_clear(buf::IOBuffer) = truncate(buf, 0) function edit_clear(s::MIState) push_undo(s) + push_kill!(s, content(s), false) || return :ignore edit_clear(buffer(s)) refresh_line(s) :edit_clear diff --git a/test/lineedit.jl b/test/lineedit.jl index e6b50cb14e402..db08ac64c2bc4 100644 --- a/test/lineedit.jl +++ b/test/lineedit.jl @@ -300,14 +300,14 @@ seek(buf,0) buf = IOBuffer("type X\n ") seekend(buf) -@test LineEdit.edit_delete_prev_word(buf) +@test !isempty(LineEdit.edit_delete_prev_word(buf)) @test position(buf) == 5 @test buf.size == 5 @test content(buf) == "type " buf = IOBuffer("4 +aaa+ x") seek(buf,8) -@test LineEdit.edit_delete_prev_word(buf) +@test !isempty(LineEdit.edit_delete_prev_word(buf)) @test position(buf) == 3 @test buf.size == 4 @test content(buf) == "4 +x" @@ -316,11 +316,11 @@ buf = IOBuffer("x = func(arg1,arg2 , arg3)") seekend(buf) LineEdit.char_move_word_left(buf) @test position(buf) == 21 -@test LineEdit.edit_delete_prev_word(buf) +@test !isempty(LineEdit.edit_delete_prev_word(buf)) @test content(buf) == "x = func(arg1,arg3)" -@test LineEdit.edit_delete_prev_word(buf) +@test !isempty(LineEdit.edit_delete_prev_word(buf)) @test content(buf) == "x = func(arg3)" -@test LineEdit.edit_delete_prev_word(buf) +@test !isempty(LineEdit.edit_delete_prev_word(buf)) @test content(buf) == "x = arg3)" # Unicode combining characters @@ -617,6 +617,29 @@ end LineEdit.edit_kill_line(s) @test s.kill_ring[end] == "çhing" @test s.kill_idx == 3 + # repetition (concatenation of killed strings + edit_insert(s, "A B C") + LineEdit.edit_delete_prev_word(s) + s.key_repeats = 1 + LineEdit.edit_delete_prev_word(s) + s.key_repeats = 0 + @test s.kill_ring[end] == "B C" + LineEdit.edit_yank(s) + LineEdit.edit_werase(s) + @test s.kill_ring[end] == "C" + s.key_repeats = 1 + LineEdit.edit_werase(s) + s.key_repeats = 0 + @test s.kill_ring[end] == "B C" + LineEdit.edit_yank(s) + LineEdit.edit_move_word_left(s) + LineEdit.edit_move_word_left(s) + LineEdit.edit_delete_next_word(s) + @test s.kill_ring[end] == "B" + s.key_repeats = 1 + LineEdit.edit_delete_next_word(s) + s.key_repeats = 0 + @test s.kill_ring[end] == "B C" end @testset "undo" begin From 74b8bb18573662d3e13e35f29a19706f2b522cb2 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Thu, 7 Sep 2017 18:14:01 +0200 Subject: [PATCH 06/11] use global constants to store the defaults (to avoid having to overwrite the beep function) --- base/repl/LineEdit.jl | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 412fd6e8f7a9d..d91cf04255933 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -117,21 +117,29 @@ complete_line(c::EmptyCompletionProvider, s) = [], true, true terminal(s::IO) = s terminal(s::PromptState) = s.terminal +# NOTE: this would better be Threads.Atomic(0.0), but not supported on some platforms const beeping = Ref(0.0) -function beep(s::PromptState, duration=0.2, blink=0.2, maxduration=1.0; - colors=[Base.text_colors[:light_black]], - use_current::Bool=true, - underline::Bool=false) - isinteractive() || return - beeping[] = min(beeping[]+duration, maxduration) +# these may be better stored in Prompt or PromptState +const BEEP_DURATION = Ref(0.2) +const BEEP_BLINK = Ref(0.2) +const BEEP_MAXDURATION = Ref(1.0) +const BEEP_COLORS = ["\e[90m"] # gray (text_colors not yet available) +const BEEP_USE_CURRENT = Ref(true) + +function beep(s::PromptState, duration::Real=BEEP_DURATION[], blink::Real=BEEP_BLINK[], + maxduration::Real=BEEP_MAXDURATION[]; + colors=BEEP_COLORS, use_current::Bool=BEEP_USE_CURRENT[]) + isinteractive() || return # some tests fail on some platforms + beeping[] = min(beeping[] + duration, maxduration) @async begin trylock(BEEP_LOCK) || return orig_prefix = s.p.prompt_prefix + colors = Base.copymutable(colors) use_current && push!(colors, orig_prefix) i = 0 while beeping[] > 0.0 - prefix = colors[mod1(i+=1, end)] * Base.text_colors[:underline]^underline + prefix = colors[mod1(i+=1, end)] s.p.prompt_prefix = prefix refresh_multi_line(s) sleep(blink) @@ -145,14 +153,16 @@ function beep(s::PromptState, duration=0.2, blink=0.2, maxduration=1.0; end function cancel_beep(s::PromptState) - beeping[] = 0.0 # wait till beeping finishes - while !trylock(BEEP_LOCK) sleep(.05) end + while !trylock(BEEP_LOCK) + beeping[] = 0.0 + sleep(.05) + end unlock(BEEP_LOCK) end -beep(s) = nothing -cancel_beep(s) = nothing +beep(::ModeState) = nothing +cancel_beep(::ModeState) = nothing for f in [:terminal, :on_enter, :add_history, :buffer, :(Base.isempty), :replace_line, :refresh_multi_line, :input_string, :update_display_buffer, From f31aac5483276a6e393d3b20454b74135cdef3d7 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Thu, 7 Sep 2017 18:26:38 +0200 Subject: [PATCH 07/11] try once more to use atomic variables --- base/repl/LineEdit.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index d91cf04255933..926feb0e88ae3 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -117,8 +117,7 @@ complete_line(c::EmptyCompletionProvider, s) = [], true, true terminal(s::IO) = s terminal(s::PromptState) = s.terminal -# NOTE: this would better be Threads.Atomic(0.0), but not supported on some platforms -const beeping = Ref(0.0) +const beeping = Threads.Atomic{Float64}(0.0) # these may be better stored in Prompt or PromptState const BEEP_DURATION = Ref(0.2) @@ -131,7 +130,8 @@ function beep(s::PromptState, duration::Real=BEEP_DURATION[], blink::Real=BEEP_B maxduration::Real=BEEP_MAXDURATION[]; colors=BEEP_COLORS, use_current::Bool=BEEP_USE_CURRENT[]) isinteractive() || return # some tests fail on some platforms - beeping[] = min(beeping[] + duration, maxduration) + Threads.atomic_add!(beeping, duration) + Threads.atomic_min!(beeping, maxduration) @async begin trylock(BEEP_LOCK) || return orig_prefix = s.p.prompt_prefix @@ -143,11 +143,11 @@ function beep(s::PromptState, duration::Real=BEEP_DURATION[], blink::Real=BEEP_B s.p.prompt_prefix = prefix refresh_multi_line(s) sleep(blink) - beeping[] -= blink + Threads.atomic_sub!(beeping, blink) end s.p.prompt_prefix = orig_prefix refresh_multi_line(s) - beeping[] = 0.0 + Threads.atomic_xchg!(beeping, 0.0) unlock(BEEP_LOCK) end end @@ -155,7 +155,7 @@ end function cancel_beep(s::PromptState) # wait till beeping finishes while !trylock(BEEP_LOCK) - beeping[] = 0.0 + Threads.atomic_xchg!(beeping, 0.0) sleep(.05) end unlock(BEEP_LOCK) From 9d361934de099ad7f2fe2a19993a147a8a7bbab2 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Fri, 8 Sep 2017 10:47:57 +0200 Subject: [PATCH 08/11] Revert "try once more to use atomic variables" This reverts commit f31aac5483276a6e393d3b20454b74135cdef3d7. --- base/repl/LineEdit.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 926feb0e88ae3..d91cf04255933 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -117,7 +117,8 @@ complete_line(c::EmptyCompletionProvider, s) = [], true, true terminal(s::IO) = s terminal(s::PromptState) = s.terminal -const beeping = Threads.Atomic{Float64}(0.0) +# NOTE: this would better be Threads.Atomic(0.0), but not supported on some platforms +const beeping = Ref(0.0) # these may be better stored in Prompt or PromptState const BEEP_DURATION = Ref(0.2) @@ -130,8 +131,7 @@ function beep(s::PromptState, duration::Real=BEEP_DURATION[], blink::Real=BEEP_B maxduration::Real=BEEP_MAXDURATION[]; colors=BEEP_COLORS, use_current::Bool=BEEP_USE_CURRENT[]) isinteractive() || return # some tests fail on some platforms - Threads.atomic_add!(beeping, duration) - Threads.atomic_min!(beeping, maxduration) + beeping[] = min(beeping[] + duration, maxduration) @async begin trylock(BEEP_LOCK) || return orig_prefix = s.p.prompt_prefix @@ -143,11 +143,11 @@ function beep(s::PromptState, duration::Real=BEEP_DURATION[], blink::Real=BEEP_B s.p.prompt_prefix = prefix refresh_multi_line(s) sleep(blink) - Threads.atomic_sub!(beeping, blink) + beeping[] -= blink end s.p.prompt_prefix = orig_prefix refresh_multi_line(s) - Threads.atomic_xchg!(beeping, 0.0) + beeping[] = 0.0 unlock(BEEP_LOCK) end end @@ -155,7 +155,7 @@ end function cancel_beep(s::PromptState) # wait till beeping finishes while !trylock(BEEP_LOCK) - Threads.atomic_xchg!(beeping, 0.0) + beeping[] = 0.0 sleep(.05) end unlock(BEEP_LOCK) From ffb1e94eef493ec1a16a30bccbe8677158c56e45 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Fri, 8 Sep 2017 13:14:58 +0200 Subject: [PATCH 09/11] use cancel_beep in transition --- base/repl/LineEdit.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index d91cf04255933..443297a9af9f5 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -1871,6 +1871,7 @@ activate(m::ModalInterface, s::MIState, termbuf, term::TextTerminal) = commit_changes(t::UnixTerminal, termbuf) = write(t, take!(termbuf.out_stream)) function transition(f::Function, s::MIState, newmode) + cancel_beep(s) if newmode === :abort s.aborted = true return From a4ba29884fe7542d685e8ef31c7e6c17eddb6e89 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Fri, 8 Sep 2017 14:30:21 +0200 Subject: [PATCH 10/11] use only one lock: now "beeping" will stop when other actions happen Using 2 locks was probably overkill, for non-essential feature. The beeping/blinking will continue though when edit_insert bypasses refresh_multi_line and writes directly to the terminal. Also, moved the lock and beeping variable inside PromptState. --- base/repl/LineEdit.jl | 50 +++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 443297a9af9f5..5a53e9cfa640b 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -9,9 +9,6 @@ import ..Terminals: raw!, width, height, cmove, getX, import Base: ensureroom, peek, show, AnyDict, position -const REFRESH_LOCK = Threads.SpinLock() -const BEEP_LOCK = Threads.SpinLock() - abstract type TextInterface end abstract type ModeState end @@ -76,6 +73,9 @@ mutable struct PromptState <: ModeState # indentation of lines which do not include the prompt # if negative, the width of the prompt is used indent::Int + refresh_lock::Threads.AbstractLock + # this would better be Threads.Atomic{Float64}, but not supported on some platforms + beeping::Float64 end setmark(s) = mark(buffer(s)) @@ -117,10 +117,8 @@ complete_line(c::EmptyCompletionProvider, s) = [], true, true terminal(s::IO) = s terminal(s::PromptState) = s.terminal -# NOTE: this would better be Threads.Atomic(0.0), but not supported on some platforms -const beeping = Ref(0.0) -# these may be better stored in Prompt or PromptState +# these may be better stored in Prompt or LineEditREPL const BEEP_DURATION = Ref(0.2) const BEEP_BLINK = Ref(0.2) const BEEP_MAXDURATION = Ref(1.0) @@ -131,34 +129,34 @@ function beep(s::PromptState, duration::Real=BEEP_DURATION[], blink::Real=BEEP_B maxduration::Real=BEEP_MAXDURATION[]; colors=BEEP_COLORS, use_current::Bool=BEEP_USE_CURRENT[]) isinteractive() || return # some tests fail on some platforms - beeping[] = min(beeping[] + duration, maxduration) + s.beeping = min(s.beeping + duration, maxduration) @async begin - trylock(BEEP_LOCK) || return + trylock(s.refresh_lock) || return orig_prefix = s.p.prompt_prefix colors = Base.copymutable(colors) use_current && push!(colors, orig_prefix) i = 0 - while beeping[] > 0.0 + while s.beeping > 0.0 prefix = colors[mod1(i+=1, end)] s.p.prompt_prefix = prefix - refresh_multi_line(s) + refresh_multi_line(s, beeping=true) sleep(blink) - beeping[] -= blink + s.beeping -= blink end s.p.prompt_prefix = orig_prefix - refresh_multi_line(s) - beeping[] = 0.0 - unlock(BEEP_LOCK) + refresh_multi_line(s, beeping=true) + s.beeping = 0.0 + unlock(s.refresh_lock) end end function cancel_beep(s::PromptState) # wait till beeping finishes - while !trylock(BEEP_LOCK) - beeping[] = 0.0 + while !trylock(s.refresh_lock) + s.beeping = 0.0 sleep(.05) end - unlock(BEEP_LOCK) + unlock(s.refresh_lock) end beep(::ModeState) = nothing @@ -278,11 +276,10 @@ prompt_string(p::Prompt) = prompt_string(p.prompt) prompt_string(s::AbstractString) = s prompt_string(f::Function) = Base.invokelatest(f) -refresh_multi_line(s::ModeState) = refresh_multi_line(terminal(s), s) -refresh_multi_line(termbuf::TerminalBuffer, s::ModeState) = refresh_multi_line(termbuf, terminal(s), s) -refresh_multi_line(termbuf::TerminalBuffer, term, s::ModeState) = (@assert term == terminal(s); refresh_multi_line(termbuf,s)) +refresh_multi_line(s::ModeState; kw...) = refresh_multi_line(terminal(s), s; kw...) +refresh_multi_line(termbuf::TerminalBuffer, s::ModeState; kw...) = refresh_multi_line(termbuf, terminal(s), s; kw...) +refresh_multi_line(termbuf::TerminalBuffer, term, s::ModeState; kw...) = (@assert term == terminal(s); refresh_multi_line(termbuf,s; kw...)) function refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, buf, state::InputAreaState, prompt = ""; indent = 0) - while !trylock(REFRESH_LOCK) sleep(.01) end _clear_input_area(termbuf, state) cols = width(terminal) @@ -346,7 +343,6 @@ function refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, buf #columns are 1 based cmove_col(termbuf, curs_pos + 1) - unlock(REFRESH_LOCK) # Updated cur_row,curs_row return InputAreaState(cur_row, curs_row) end @@ -1354,9 +1350,11 @@ function show(io::IO, s::PrefixSearchState) isdefined(s,:mi) ? s.mi : "no MI") end -refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, - s::Union{PromptState,PrefixSearchState}) = s.ias = - refresh_multi_line(termbuf, terminal, buffer(s), s.ias, s, indent = s.indent) +function refresh_multi_line(termbuf::TerminalBuffer, terminal::UnixTerminal, + s::Union{PromptState,PrefixSearchState}; beeping=false) + beeping || cancel_beep(s) + s.ias = refresh_multi_line(termbuf, terminal, buffer(s), s.ias, s, indent = s.indent) +end input_string(s::PrefixSearchState) = String(take!(copy(s.response_buffer))) @@ -1929,7 +1927,7 @@ run_interface(::Prompt) = nothing init_state(terminal, prompt::Prompt) = PromptState(terminal, prompt, IOBuffer(), IOBuffer[], 1, InputAreaState(1, 1), - #=indent(spaces)=# -1) + #=indent(spaces)=# -1, Threads.SpinLock(), 0.0) function init_state(terminal, m::ModalInterface) s = MIState(m, m.modes[1], false, Dict{Any,Any}()) From ed63ec6fc8d35e4c0fa51caf7c7dda82a468ca04 Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Fri, 8 Sep 2017 15:12:35 +0200 Subject: [PATCH 11/11] fix deadlock (sort of) in complete_line --- base/repl/LineEdit.jl | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 5a53e9cfa640b..5ab266d4eee8b 100644 --- a/base/repl/LineEdit.jl +++ b/base/repl/LineEdit.jl @@ -221,16 +221,19 @@ end # Prompt Completions function complete_line(s::MIState) - complete_line(state(s), s.key_repeats) - refresh_line(s) - :complete_line + if complete_line(state(s), s.key_repeats) + refresh_line(s) + :complete_line + else + beep(s) + :ignore + end end function complete_line(s::PromptState, repeats) completions, partial, should_complete = complete_line(s.p.complete, s) - if isempty(completions) - beep(s) - elseif !should_complete + isempty(completions) && return false + if !should_complete # should_complete is false for cases where we only want to show # a list of possible completions but not complete, e.g. foo(\t show_completions(s, completions) @@ -251,6 +254,7 @@ function complete_line(s::PromptState, repeats) show_completions(s, completions) end end + true end clear_input_area(terminal, s) = (_clear_input_area(terminal, s.ias); s.ias = InputAreaState(0, 0))