fix(messages): dangling progress-messages #41222

Problem:
Some builtin features emit progress-messages which never "complete".
- On failure, `:write` does not complete the progress-msg it started.
- ins-completion never ends its "Scanning..." message.

Solution:
- `buf_write()` emits "failed" status on failure.
- `ins_compl_stop()` ends the completion one.

runtime/doc/message.txt

@@ -891,6 +891,10 @@ Create or update a progress-message by calling |nvim_echo()| with
 `nvim_echo`. You can specify the id when calling `nvim_echo` to update an
 existing progress-message.
 
+Note: To avoid a "dangling" progress-message, always emit a non-"running"
+`status`, when your task is done; otherwise consumers (including Nvim itself)
+tracking the task by its message-id cannot know when the task is done.
+
 Events: ~
     • msg_show |ui-messages| event is fired for ext-ui upon creation/update of
       a progress-message

src/nvim/bufwrite.c

@@ -560,18 +560,25 @@ static inline Error_T set_err_arg(const char *msg, int arg)
   return (Error_T){ .num = NULL, .msg = (char *)msg, .arg = arg };
 }
 
-static void emit_err(Error_T *e)
+/// Shows a write error msg.  IObuff must hold the quoted file name.
+///
+/// @param msg_id  Ends the progress-message with "failed" status. NULL to show a plain error msg.
+static void emit_err(Error_T *e, char *msg_id)
 {
-  if (e->num != NULL) {
-    if (e->arg != 0) {
-      semsg("%s: %s%s: %s", e->num, IObuff, e->msg, os_strerror(e->arg));
-    } else {
-      semsg("%s: %s%s", e->num, IObuff, e->msg);
-    }
+  char errmsg[IOSIZE];
+  if (e->num != NULL && e->arg != 0) {
+    vim_snprintf(errmsg, IOSIZE, "%s: %s%s: %s", e->num, IObuff, e->msg, os_strerror(e->arg));
+  } else if (e->num != NULL) {
+    vim_snprintf(errmsg, IOSIZE, "%s: %s%s", e->num, IObuff, e->msg);
   } else if (e->arg != 0) {
-    semsg(e->msg, os_strerror(e->arg));
+    vim_snprintf(errmsg, IOSIZE, e->msg, os_strerror(e->arg));
   } else {
-    emsg(e->msg);
+    xstrlcpy(errmsg, e->msg, IOSIZE);
+  }
+  if (msg_id != NULL) {
+    msg_progress(errmsg, msg_id, "failed", HLF_E, false, false, true);
+  } else {
+    emsg(errmsg);
   }
   if (e->alloc) {
     xfree(e->msg);
@@ -1664,9 +1671,8 @@ restore_backup:
 #endif
   if (!filtering) {
     add_quoted_fname(IObuff, IOSIZE, buf, fname);
-    // Append the filename (without trailing char) to the message ID.
-    char msg_id[IOSIZE + 14] = "nvim.bufwrite ";
-    xstrlcat(msg_id, IObuff, 14 + strlen(IObuff));
+    char msg_id[bufwrite_msg_id_size];
+    msg_id_for_bufwrite(msg_id, buf, fname);
     bool insert_space = false;
     if (write_info.bw_conv_error) {
       xstrlcat(IObuff, _(" CONVERSION ERROR"), IOSIZE);
@@ -1707,7 +1713,7 @@ restore_backup:
     }
     // Hide cursor while emitting "written" message, so cursor doesn't flicker in cmdline. #25974
     ui_busy_start();
-    set_keep_msg(msg_progress(IObuff, msg_id, "success", 0, true, true), 0);
+    set_keep_msg(msg_progress(IObuff, msg_id, "success", 0, true, true, false), 0);
     ui_busy_stop();
   }
 
@@ -1807,13 +1813,16 @@ nofail:
   os_free_acl(acl);
 
   if (err.msg != NULL) {
-    // - 100 to save some space for further error message
 #ifndef UNIX
-    add_quoted_fname(IObuff, IOSIZE - 100, buf, sfname);
+    char *errname = sfname;
 #else
-    add_quoted_fname(IObuff, IOSIZE - 100, buf, fname);
+    char *errname = fname;
 #endif
-    emit_err(&err);
+    // - 100 to save some space for further error message.
+    add_quoted_fname(IObuff, IOSIZE - 100, buf, errname);
+    char msg_id[bufwrite_msg_id_size];  // End the progress-msg started by filemess_progress().
+    msg_id_for_bufwrite(msg_id, buf, errname);
+    emit_err(&err, filtering ? NULL : msg_id);
 
     retval = FAIL;
     if (end == 0) {

src/nvim/fileio.c

@@ -142,10 +142,9 @@ static void filemess(buf_T *buf, char *name, char *s, bool progress)
   msg_scrolled_ign = true;
   // may truncate the message to avoid a hit-return prompt
   if (progress) {
-    // Append the filename to the message ID.
-    char msg_id[IOSIZE + 14] = "nvim.bufwrite ";
-    xstrlcat(msg_id, IObuff, 14 + strlen(IObuff));
-    msg_progress(IObuff, msg_id, "running", 0, false, true);
+    char msg_id[bufwrite_msg_id_size];
+    msg_id_for_bufwrite(msg_id, buf, name);
+    msg_progress(IObuff, msg_id, "running", 0, false, true, false);
   } else {
     msg_outtrans(msg_may_trunc(false, IObuff), 0, false);
   }

src/nvim/indent.c

@@ -973,7 +973,7 @@ void op_reindent(oparg_T *oap, Indenter how)
         // Restore cursor to avoid redrawing curwin in msg_show callback.
         linenr_T save_lnum = curwin->w_cursor.lnum;
         curwin->w_cursor.lnum = start_lnum;
-        msg_progress(IObuff, "nvim.indent", "running", 0, true, false);
+        msg_progress(IObuff, "nvim.indent", "running", 0, true, false, false);
         curwin->w_cursor.lnum = save_lnum;
       }
 
@@ -1015,7 +1015,7 @@ void op_reindent(oparg_T *oap, Indenter how)
     i = oap->line_count - (i + 1);
     snprintf(IObuff, IOSIZE,
              NGETTEXT("%" PRId64 " line indented ", "%" PRId64 " lines indented ", i), (int64_t)i);
-    msg_progress(IObuff, "nvim.indent", "success", 0, true, false);
+    msg_progress(IObuff, "nvim.indent", "success", 0, true, false, false);
   }
   if ((cmdmod.cmod_flags & CMOD_LOCKMARKS) == 0) {
     // set '[ and '] marks

src/nvim/insexpand.c

@@ -2076,7 +2076,7 @@ static void ins_compl_files(int count, char **files, bool thesaurus, int flags,
     FILE *fp = os_fopen(files[i], "r");  // open dictionary file
     if (flags != DICT_EXACT && !shortmess(kShmCompletionscan) && !compl_autocomplete) {
       vim_snprintf(IObuff, IOSIZE, _("Scanning dictionary: %s"), files[i]);
-      msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true);
+      msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true, false);
     }
 
     if (fp == NULL) {
@@ -2856,6 +2856,10 @@ static bool ins_compl_stop(const int c, const int prev_mode, bool retval)
   ins_compl_free();
   compl_started = false;
   compl_matches = 0;
+  if (!shortmess(kShmCompletionscan) && !compl_autocomplete) {
+    // End the "Scanning..." progress-msg. ins_compl_show_statusmsg() already reported the result.
+    msg_progress(NULL, "nvim.completion", "success", 0, false, false, false);
+  }
   if (!shortmess(kShmCompletionmenu)) {
     msg_clr_cmdline();  // necessary for "noshowmode"
   }
@@ -3895,7 +3899,7 @@ static int process_next_cpt_value(ins_compl_next_state_T *st, int *compl_type_ar
                    : st->ins_buf->b_sfname == NULL
                    ? st->ins_buf->b_fname
                    : st->ins_buf->b_sfname);
-      msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true);
+      msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true, false);
     }
   } else if (*st->e_cpt == NUL) {
     status = INS_COMPL_CPT_END;
@@ -3929,7 +3933,7 @@ static int process_next_cpt_value(ins_compl_next_state_T *st, int *compl_type_ar
         compl_type = CTRL_X_TAGS;
         if (!shortmess(kShmCompletionscan) && !compl_autocomplete) {
           vim_snprintf(IObuff, IOSIZE, "%s", _("Scanning tags."));
-          msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true);
+          msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true, false);
         }
       }
     }

src/nvim/message.c

@@ -169,6 +169,18 @@ bool msg_id_exists(int64_t id)
   return id > 0 && id < msg_id_next;
 }
 
+/// Formats a progress-msg id for use by bufwrite.c/fileio.c.
+///
+/// @param msg_id  Buffer of `bufwrite_msg_id_size` bytes.
+void msg_id_for_bufwrite(char *msg_id, buf_T *buf, char *name)
+{
+  xstrlcpy(msg_id, "nvim.bufwrite ", bufwrite_msg_id_size);
+  size_t len = strlen(msg_id);
+  add_quoted_fname(msg_id + len, bufwrite_msg_id_size - len, buf, name);
+  // add_quoted_fname() appends a space for display; the id ends at the closing quote.
+  msg_id[strlen(msg_id) - 1] = NUL;
+}
+
 static void ui_ext_msg_set_pos(int row, bool scrolled)
 {
   char buf[MAX_SCHAR_SIZE];
@@ -1092,14 +1104,13 @@ char *msg_may_trunc(bool force, char *s)
   return s;
 }
 
-char *msg_progress(char *s, char *id, char *status, int hl_id, bool hist, bool trunc)
+/// Shows a progress-message.  A non-"running" `status` ends the message and announces the
+/// progress task as "done".
+///
+/// @param s    Message text.  NULL ends the message without showing anything.
+/// @param err  Show as an error message.
+char *msg_progress(char *s, char *id, char *status, int hl_id, bool hist, bool trunc, bool err)
 {
-  if (hist && (!trunc || ui_has(kUIMessages))) {
-    msg_hist_add(s, -1, 0);
-  }
-  if (trunc) {
-    s = msg_may_trunc(false, s);
-  }
   bool clear = false;
   MessageData data = {
     .source = cstr_as_string("nvim"),
@@ -1107,9 +1118,17 @@ char *msg_progress(char *s, char *id, char *status, int hl_id, bool hist, bool t
     .percent = -1,
   };
   HlMessage chunks = KV_INITIAL_VALUE;
-  kv_push(chunks, ((HlMessageChunk){ cstr_as_string(s), hl_id }));
+  if (s != NULL) {
+    if (hist && (!trunc || ui_has(kUIMessages))) {
+      msg_hist_add(s, -1, 0);
+    }
+    if (trunc) {
+      s = msg_may_trunc(false, s);
+    }
+    kv_push(chunks, ((HlMessageChunk){ cstr_as_string(s), hl_id }));
+  }
   msg_ext_no_fast();
-  msg_multihl(CSTR_AS_OBJ(id), chunks, "progress", false, false, &data, &clear);
+  msg_multihl(CSTR_AS_OBJ(id), chunks, "progress", false, err, &data, &clear);
   kv_destroy(chunks);
   ui_flush();
   return s;

src/nvim/message.h

@@ -9,6 +9,10 @@
 #include "nvim/keycodes_defs.h"  // IWYU pragma: keep
 #include "nvim/macros_defs.h"
 #include "nvim/message_defs.h"  // IWYU pragma: keep
+#include "nvim/os/os_defs.h"
+
+/// Buffer size for msg_id_for_bufwrite().
+enum { bufwrite_msg_id_size = MAXPATHL + 32, };
 
 /// Types of dialogs passed to do_dialog().
 enum {

test/functional/ui/messages_spec.lua

@@ -3929,6 +3929,40 @@ describe('progress-message', function()
     -- ":read" is not a write: it must not start a progress that never ends.
     command('read ' .. fname)
     assert_progress_autocmd(nil)
+
+    -- A failed write ends the progress-msg.
+    local events = exec_lua(function(f)
+      local out = {}
+      vim.api.nvim_create_autocmd('Progress', {
+        callback = function(ev)
+          table.insert(out, { id = ev.data.id, status = ev.data.status, text = ev.data.text[1] })
+        end,
+      })
+      pcall(vim.cmd.write, ('%s/nodir'):format(f))
+      return out
+    end, fname)
+    eq({ 'running', 'failed' }, { events[1].status, events[2].status })
+    eq(events[1].id, events[2].id)
+    t.matches('^E%d+:', events[2].text)
+  end)
+
+  it('emitted by ins-completion scan', function()
+    fn.writefile({ 'foobar', 'foobaz' }, 'Xdict')
+    finally(function()
+      os.remove('Xdict')
+    end)
+    exec_lua(function()
+      _G.events = {}
+      vim.api.nvim_create_autocmd('Progress', {
+        callback = function(ev)
+          table.insert(_G.events, ('%s %s'):format(ev.data.id, ev.data.status))
+        end,
+      })
+    end)
+    command('set shortmess-=C complete=kXdict')
+    feed('ifoo<C-n><Esc>')
+    -- Leaving completion ends the scan message, which otherwise stays "running" forever.
+    eq({ 'nvim.completion running', 'nvim.completion success' }, exec_lua('return _G.events'))
   end)
 
   it('tui displays progress message in proper format', function()