mirror of
https://github.com/fish-shell/fish-shell
synced 2025-01-14 05:53:59 +00:00
Allow timing-out I/O-able syntax highlighting after expanding abbreviation
It may happen that the user types an abbreviation and then hits return. Prior to this commit, we would perform a form of syntax highlighting that does not require I/O, so as to not block the user. However this could cause invalid commands to be colored as valid. More generally if the user has e.g a slow NFS mount, then syntax highlighting may lag behind the user's typing, and be incorrect at the time the user hits return. This is an unavoidable race, since proper syntax highlighting may take arbitrarily long. Introduce a new function `finish_highlighting_before_exec`, which waits for any outstanding syntax highlighting to complete, BUT has a timeout (250 milliseconds). After this, it falls back to the no-I/O variant, which colors all commands as valid and nothing as paths. Fixes #7418 Fixes #5912
This commit is contained in:
parent
c861fdadcf
commit
d3192d37a2
3 changed files with 64 additions and 25 deletions
|
@ -313,6 +313,12 @@ static bool iothread_wait_for_pending_completions(long timeout_usec) {
|
|||
return ret > 0;
|
||||
}
|
||||
|
||||
void iothread_service_completion_with_timeout(long timeout_usec) {
|
||||
if (iothread_wait_for_pending_completions(timeout_usec)) {
|
||||
iothread_service_completion();
|
||||
}
|
||||
}
|
||||
|
||||
/// At the moment, this function is only used in the test suite and in a
|
||||
/// drain-all-threads-before-fork compatibility mode that no architecture requires, so it's OK that
|
||||
/// it's terrible.
|
||||
|
|
|
@ -14,9 +14,12 @@
|
|||
/// \return the fd on which to listen for completion callbacks.
|
||||
int iothread_port();
|
||||
|
||||
/// Services one iothread completion callback.
|
||||
/// Services iothread completion callbacks.
|
||||
void iothread_service_completion();
|
||||
|
||||
/// Services completions, except does not wait more than \p timeout_usec.
|
||||
void iothread_service_completion_with_timeout(long timeout_usec);
|
||||
|
||||
/// Waits for all iothreads to terminate.
|
||||
/// \return the number of threads that were running.
|
||||
int iothread_drain_all();
|
||||
|
|
|
@ -144,6 +144,12 @@ static operation_context_t get_bg_context(const std::shared_ptr<environment_t> &
|
|||
return operation_context_t{nullptr, *env, std::move(cancel_checker)};
|
||||
}
|
||||
|
||||
/// We try to ensure that syntax highlighting completes appropriately before executing what the user typed.
|
||||
/// But we do not want it to block forever - e.g. it may hang on determining if an arbitrary argument
|
||||
/// is a path. This is how long we'll wait (in milliseconds) before giving up and performing a
|
||||
/// no-io syntax highlighting. See #7418, #5912.
|
||||
static constexpr long kHighlightTimeoutForExecutionMs = 250;
|
||||
|
||||
/// Get the debouncer for autosuggestions and background highlighting.
|
||||
/// These are deliberately leaked to avoid shutdown dtor registration.
|
||||
static debounce_t &debounce_autosuggestions() {
|
||||
|
@ -648,7 +654,11 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
|
|||
void update_autosuggestion();
|
||||
void accept_autosuggestion(bool full, bool single = false,
|
||||
move_word_style_t style = move_word_style_punctuation);
|
||||
void super_highlight_me_plenty(bool no_io = false);
|
||||
void super_highlight_me_plenty();
|
||||
|
||||
/// Finish up any outstanding syntax highlighting, before execution.
|
||||
/// This plays some tricks to not block on I/O for too long.
|
||||
void finish_highlighting_before_exec();
|
||||
|
||||
void highlight_complete(highlight_result_t result);
|
||||
void exec_mode_prompt();
|
||||
|
@ -2373,29 +2383,47 @@ static std::function<highlight_result_t(void)> get_highlight_performer(parser_t
|
|||
}
|
||||
|
||||
/// Highlight the command line in a super, plentiful way.
|
||||
/// \param no_io if true, do a highlight that does not perform I/O, synchronously. If false, perform
|
||||
/// an asynchronous highlight in the background, which may perform disk I/O.
|
||||
void reader_data_t::super_highlight_me_plenty(bool no_io) {
|
||||
void reader_data_t::super_highlight_me_plenty() {
|
||||
if (!conf.highlight_ok) return;
|
||||
|
||||
const editable_line_t *el = &command_line;
|
||||
|
||||
// Do nothing if this text is already in flight.
|
||||
const editable_line_t *el = &command_line;
|
||||
if (el->text() == in_flight_highlight_request) return;
|
||||
in_flight_highlight_request = el->text();
|
||||
|
||||
FLOG(reader_render, L"Highlighting");
|
||||
auto highlight_performer = get_highlight_performer(parser(), el->text(), !no_io);
|
||||
if (no_io) {
|
||||
// Highlighting without IO, we just do it.
|
||||
highlight_complete(highlight_performer());
|
||||
} else {
|
||||
// Highlighting including I/O proceeds in the background.
|
||||
auto shared_this = this->shared_from_this();
|
||||
debounce_highlighting().perform(highlight_performer,
|
||||
[shared_this](highlight_result_t result) {
|
||||
shared_this->highlight_complete(std::move(result));
|
||||
});
|
||||
auto highlight_performer = get_highlight_performer(parser(), el->text(), true /* io_ok */);
|
||||
auto shared_this = this->shared_from_this();
|
||||
debounce_highlighting().perform(highlight_performer, [shared_this](highlight_result_t result) {
|
||||
shared_this->highlight_complete(std::move(result));
|
||||
});
|
||||
}
|
||||
|
||||
void reader_data_t::finish_highlighting_before_exec() {
|
||||
if (!conf.highlight_ok) return;
|
||||
if (in_flight_highlight_request.empty()) return;
|
||||
|
||||
// We have an in-flight highlight request scheduled.
|
||||
// Wait for its completion to run, but not forever.
|
||||
namespace sc = std::chrono;
|
||||
auto now = sc::steady_clock::now();
|
||||
auto deadline = now + sc::milliseconds(kHighlightTimeoutForExecutionMs);
|
||||
while (now < deadline) {
|
||||
long timeout_usec = sc::duration_cast<sc::microseconds>(deadline - now).count();
|
||||
iothread_service_completion_with_timeout(timeout_usec);
|
||||
|
||||
// Note iothread_service_completion_with_timeout will reentrantly modify us,
|
||||
// by invoking a completion.
|
||||
if (in_flight_highlight_request.empty()) break;
|
||||
now = sc::steady_clock::now();
|
||||
}
|
||||
|
||||
if (!in_flight_highlight_request.empty()) {
|
||||
// We did not complete before the deadline.
|
||||
// Give up and highlight without I/O.
|
||||
const editable_line_t *el = &command_line;
|
||||
auto highlight_no_io = get_highlight_performer(parser(), el->text(), false /* io not ok */);
|
||||
this->highlight_complete(highlight_no_io());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -3084,13 +3112,12 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
|
|||
if (command_test_result == 0 || command_test_result == PARSER_TEST_INCOMPLETE) {
|
||||
// This command is valid, but an abbreviation may make it invalid. If so, we
|
||||
// will have to test again.
|
||||
bool abbreviation_expanded = expand_abbreviation_as_necessary(1);
|
||||
if (abbreviation_expanded && conf.syntax_check_ok) {
|
||||
command_test_result = reader_shell_test(parser(), el->text());
|
||||
// If the command is OK, then we're going to execute it. We still want to do
|
||||
// syntax highlighting on this newly changed command, but a synchronous variant
|
||||
// that performs no I/O, so as not to block the user.
|
||||
if (command_test_result == 0) super_highlight_me_plenty(true);
|
||||
if (expand_abbreviation_as_necessary(1)) {
|
||||
// Trigger syntax highlighting as we are likely about to execute this command.
|
||||
this->super_highlight_me_plenty();
|
||||
if (conf.syntax_check_ok) {
|
||||
command_test_result = reader_shell_test(parser(), el->text());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -3760,6 +3787,9 @@ maybe_t<wcstring> reader_data_t::readline(int nchars_or_0) {
|
|||
// user presses enter.
|
||||
if (this->is_repaint_needed() || conf.in != STDIN_FILENO) this->layout_and_repaint(L"prepare to execute");
|
||||
|
||||
// Finish any outstanding syntax highlighting (but do not wait forever).
|
||||
finish_highlighting_before_exec();
|
||||
|
||||
// Emit a newline so that the output is on the line after the command.
|
||||
// But do not emit a newline if the cursor has wrapped onto a new line all its own - see #6826.
|
||||
if (!screen.cursor_is_wrapped_to_own_line()) {
|
||||
|
|
Loading…
Reference in a new issue