diff --git a/functions/fish_prompt.fish b/functions/fish_prompt.fish index 87016d9..7cc06ad 100644 --- a/functions/fish_prompt.fish +++ b/functions/fish_prompt.fish @@ -16,12 +16,17 @@ set -g $prompt_var set -q _tide_prompt_tmpdir || set -g _tide_prompt_tmpdir (mktemp -d) set -g _tide_prompt_tmpfile $_tide_prompt_tmpdir/prompt -# Bumped on every dispatch and stamped into each job's output, so a job that -# finishes after a newer one has already been dispatched -- e.g. a slow -# in-repo render outliving a fast render for the directory `cd`ed into next -# -- can be recognized as superseded and dropped instead of overwriting -# fresher content. -set -q _tide_render_gen || set -g _tide_render_gen 0 +# Bumped once per prompt cycle and stamped into $_tide_repaint every time a +# repaint is requested. Without it, a repaint requested while a command is +# still running -- a render landing, or a terminal resize, during a long +# command -- would be mistaken for a repaint of the prompt drawn next, whose +# render would then be skipped, leaving pre-command content on screen until +# something else redrew it. +set -q _tide_cycle || set -g _tide_cycle 0 + +function _tide_next_prompt_cycle --on-event fish_postexec + set -g _tide_cycle (math $_tide_cycle + 1) +end set_color normal | read -l color_normal status fish-path | read -l fish_path @@ -32,16 +37,17 @@ end # _tide_repaint prevents us from creating a second background job function _tide_refresh_prompt --on-signal SIGUSR1 --inherit-variable prompt_var - set -l rendered (cat $_tide_prompt_tmpfile 2>/dev/null) - # First line is the generation this job was dispatched at (see - # _tide_dispatch_render) -- a mismatch means a newer job has since been - # dispatched, so this result is stale and superseded; drop it rather - # than clobbering $prompt_var with old content. No repaint here is - # correct: the newer job has either already applied its own result or - # will signal on its own once it finishes. - test "$rendered[1]" = "$_tide_render_gen" || return - set -g $prompt_var $rendered[2..] - set -g _tide_repaint + # Every job publishes under its own pid (see _tide_dispatch_render), so + # only the newest job's file is ever read: a superseded job that finishes + # late writes somewhere else entirely, where it can neither be read here + # nor overwrite fresher content. Nothing there yet means this signal came + # from such a straggler while the current job is still rendering -- drop + # it and wait, the current job signals for itself once it finishes. + set -q _tide_last_pid || return + set -l rendered (cat $_tide_prompt_tmpfile.$_tide_last_pid 2>/dev/null) + set -q rendered[1] || return + set -g $prompt_var $rendered + set -g _tide_repaint $_tide_cycle commandline -f repaint end @@ -49,7 +55,7 @@ end # alone -- in synchronous-fallback mode the tmpfile is never written, so # reading it here would blank the prompt. function _tide_repaint_on_resize --on-variable COLUMNS - set -g _tide_repaint + set -g _tide_repaint $_tide_cycle commandline -f repaint end @@ -75,33 +81,40 @@ function _tide_dispatch_render --inherit-variable prompt_var --inherit-variable return end - set -g _tide_render_gen (math $_tide_render_gen + 1) - - # The job renders into a private scratch file (suffixed with its own - # pid) and atomically renames it over the shared tmpfile, so the - # SIGUSR1 handler can never read a partial write from a newer job. Its - # first line is stamped with the generation dispatched here (baked in - # as a literal below, not read back from the variable, since the job - # is a separate process) so _tide_refresh_prompt can recognize and - # drop a result superseded by a since-dispatched job. The tmpfile path - # crosses into the job string-escaped and is expanded as a variable - # there, never re-parsed as syntax, so any TMPDIR is safe. + # Removing the previous job's files is left to the job below, so that fork + # lands in the background instead of on the interactive path. + set -l rm_stale + if set -q _tide_last_pid + set -l prev (string escape -- $_tide_prompt_tmpfile.$_tide_last_pid) + set rm_stale "command rm -f $prev 2>/dev/null" + + # `.part` is renamed away the moment a job publishes, so one still + # sitting there means the previous job never finished -- only then is + # there anything to kill, and only then do we pay for the fork. Its + # scratch file is only safe to delete once that kill has landed: + # deleting it under a job that is still rendering would leave that + # job's rename with nothing to rename. + if test -e $_tide_prompt_tmpfile.$_tide_last_pid.part && command kill $_tide_last_pid 2>/dev/null + set -l prev_part (string escape -- $_tide_prompt_tmpfile.$_tide_last_pid.part) + set rm_stale "command rm -f $prev $prev_part 2>/dev/null" + end + end + + # The job renders into a private scratch file and atomically renames it to + # a path named after its own pid, so the SIGUSR1 handler can never read a + # partial write, and a superseded job can never overwrite the result of a + # job dispatched after it. Paths cross into the job string-escaped -- and + # the tmpfile is expanded there as a variable, never re-parsed as syntax + # -- so any TMPDIR is safe. $fish_path -c "set _tide_pipestatus $_tide_pipestatus set _tide_parent_dirs $_tide_parent_dirs set _tide_prompt_tmpfile "(string escape -- $_tide_prompt_tmpfile)" -echo $_tide_render_gen >\$_tide_prompt_tmpfile.\$fish_pid -PATH="(string escape "$PATH")" CMD_DURATION=$CMD_DURATION fish_key_bindings=$fish_key_bindings fish_bind_mode=$fish_bind_mode $argv[1] >>\$_tide_prompt_tmpfile.\$fish_pid -command mv -f \$_tide_prompt_tmpfile.\$fish_pid \$_tide_prompt_tmpfile -command kill -s USR1 $fish_pid 2>/dev/null" & +PATH="(string escape "$PATH")" CMD_DURATION=$CMD_DURATION fish_key_bindings=$fish_key_bindings fish_bind_mode=$fish_bind_mode $argv[1] >\$_tide_prompt_tmpfile.\$fish_pid.part +command mv -f \$_tide_prompt_tmpfile.\$fish_pid.part \$_tide_prompt_tmpfile.\$fish_pid 2>/dev/null +command kill -s USR1 $fish_pid 2>/dev/null +$rm_stale" & builtin disown - # A completed job has already mv'd its scratch file away, so its - # existence means the previous job is still running or died mid-render -- - # only then is there anything to kill/clean up, and only then do we pay - # for the two external forks. - test -e $_tide_prompt_tmpfile.$_tide_last_pid && - command kill $_tide_last_pid 2>/dev/null && - command rm -f $_tide_prompt_tmpfile.$_tide_last_pid set -g _tide_last_pid $last_pid if not set -q _tide_signal_confirmed @@ -131,7 +144,9 @@ if contains newline $_tide_left_items # two line prompt initialization eval " function fish_prompt set -lx _tide_status \$status - _tide_pipestatus=\$pipestatus if not set -e _tide_repaint + _tide_pipestatus=\$pipestatus if test \"\$_tide_repaint\" = \"\$_tide_cycle\" + set -e _tide_repaint + else if not contains -- --final-rendering \$argv _tide_dispatch_render _tide_2_line_prompt end @@ -168,7 +183,9 @@ else # one line prompt initialization eval " function fish_prompt set -lx _tide_status \$status - _tide_pipestatus=\$pipestatus if not set -e _tide_repaint + _tide_pipestatus=\$pipestatus if test \"\$_tide_repaint\" = \"\$_tide_cycle\" + set -e _tide_repaint + else if not contains -- --final-rendering \$argv _tide_dispatch_render _tide_1_line_prompt end diff --git a/tests/fish_prompt.test.fish b/tests/fish_prompt.test.fish index eaa5f58..49dfe1b 100644 --- a/tests/fish_prompt.test.fish +++ b/tests/fish_prompt.test.fish @@ -114,4 +114,71 @@ echo stderr-lines (count <$stderr_log) # CHECK: stderr-lines 0 command rm -f $stderr_log +# A superseded job that finishes late must not be able to hide or destroy the +# newest job's result. Every job publishes under its own pid, so this can be +# staged directly: write what the current job published, then write what a +# slower job dispatched *earlier* publishes afterwards, then deliver the +# signal. The late write lands somewhere the handler never reads, so the +# order of the two writes cannot matter -- which is the whole point, since +# with one shared file the later write won. +fish -i -c ' + false + fish_prompt >/dev/null + for i in (seq 1 50) + set -q _tide_repaint && break + sleep 0.01 + end + echo fresh-content >$_tide_prompt_tmpfile.$_tide_last_pid + echo stale-junk >$_tide_prompt_tmpfile.999999 + command kill -s USR1 $fish_pid + sleep 0.1 + echo applied: (_tide_decolor (fish_prompt)) +' /dev/null + end + sleep 0.5 +' $stderr_log +echo overlap-stderr-lines (count <$stderr_log) +# CHECK: overlap-stderr-lines 0 +command rm -f $stderr_log + +# A repaint requested in one prompt cycle must not suppress the render of the +# next one. Both handlers can fire while a command is still running -- an +# async render landing, or a terminal resize -- and the prompt drawn after +# that command has to show the command's own status, not the previous one's. +# `fish_postexec` doesn't fire for statements inside `fish -c`, so the cycle +# boundary is emitted by hand. +for source in render resize + fish -i -c " + false + fish_prompt >/dev/null + for i in (seq 1 50) + set -q _tide_repaint && break + sleep 0.01 + end + test $source = resize && _tide_repaint_on_resize + + emit fish_postexec + true; fish_prompt >/dev/null + for i in (seq 1 50) + test \"\$_tide_repaint\" = \"\$_tide_cycle\" && break + sleep 0.01 + end + echo $source: (_tide_decolor (fish_prompt)) +" /dev/null + test "$_tide_last_pid" = "$pid_before" && + echo transient-dispatched-no || echo transient-dispatched-yes ' $stderr_log # CHECK: normal: # CHECK: {{.*}}╭─{{.*}}─╮{{.*}} @@ -44,6 +55,7 @@ fish -i -c ' # CHECK: {{\x1b\[0J}}❯ # CHECK: transient-right: # CHECK: transient-right-end +# CHECK: transient-dispatched-no echo stderr-lines (count <$stderr_log) # CHECK: stderr-lines 0