Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

re-enable terminal's beep and use... something else! #23201

Merged
merged 15 commits into from
Sep 10, 2017
Merged

Conversation

rfourquet
Copy link
Member

@rfourquet rfourquet commented Aug 10, 2017

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.

Alternative to #9710, which discuss the merits of having a visual feed-back sometimes (e.g. when <TAB> shows no completion, is it that the current word is valid, or that there are no completion for it?)

The idea here is to make the prompt blink. I like it sober, so it blinks once by default, alternating with :light_black color (can be customized). This is done asynchronously, so that it's not blocking. If a beep is requested when already "beeping", the duration (by default 0.2s) adds up, to a maximum of maxduration. blink sets the duration of a blink, but maybe should be changed to mean the number of blinks.

Here is a small screenshot: first the word "qwe<TAB>" is typed and then it blinks because there are no completion. Then <BACKSPACE> is held so that blinking accumulates to the maximum value, then the repl is used (and complection used on "function") while blinking: beep3

I'm not quite sure my locking scheme is OK. Will have to check again (and I could use some advices...).

EDIT: it you want to try, it's easy to customize, e.g.

Base.LineEdit.beep(s::Base.LineEdit.PromptState) =
    Base.LineEdit.beep(s, 0.8, 0.1, 2.0,
                       colors=[Base.text_colors[c] for c in (:yellow, :magenta, :red)],
                       use_current=false, # don't use the current color in `colors` above
                       underline=true)

(Am still in awe of 265 solved...)

@rfourquet rfourquet added the REPL Julia's REPL (Read Eval Print Loop) label Aug 10, 2017
@rfourquet
Copy link
Member Author

Does someone have a clue what causes the Appveyor errors? ("ArgumentError: stream is closed or unusable" in the "replcompletions" test). No such error in with Travis and on my box.

@StefanKarpinski
Copy link
Member

Maybe they use a terminal emulator to capture output that doesn't like something this PR is printing?

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.
It's probably not crucial for this functionality to have atomicity.
@rfourquet
Copy link
Member Author

So tests pass now :) I will merge in a couple of days if no objection or no more comments.

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.
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.
@rfourquet
Copy link
Member Author

rfourquet commented Sep 10, 2017

I am going to merge this without waiting for CI as it runned already yesterday, but there was now a very trivial merge conflict (see last merge commit), which doesn't warrant using CI resources. EDIT: oups, CI still runs after merge... but the queue on travis doesn't look big, so I guess it's ok.

I decided to simplify a bit the functionality, because using two different locks for this seemed overkill: if the user makes an action when it's beeping, the beeping is canceled before executing the action.

It's now possible to play with the options by manipulating global Ref variables, e.g. LineEdit.BEEP_BLINK[] = .08. Using configuration options like in #23637 should be supported, but it's inconvenient for playing with them. Please comment here for any suggestion of better defaults or improvements.

@rfourquet rfourquet merged commit 8c2fc37 into master Sep 10, 2017
@rfourquet rfourquet deleted the rf/repl-beep branch September 10, 2017 08:51
@giordano
Copy link
Contributor

This is awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants