feat(progress): better default format + history sync #35533

Problem:
The default progress message doesn't account for
message-status. Also, the title and percent sections don't get written
to history. And progress percent is hard to find with variable length messages.

Solution:
Apply highlighting on Title based on status. And sync the formated msg
in history too. Also updates the default progress message format to
{title}: {percent}% msg

runtime/doc/news.txt

@@ -204,6 +204,7 @@ HIGHLIGHTS
 • |hl-DiffTextAdd| highlights added text within a changed line.
 • |hl-StderrMsg| |hl-StdoutMsg|
 • |hl-SnippetTabstopActive| highlights the currently active tabstop.
+• |hl-OkMsg| highlights success messages.
 
 LSP
 

runtime/doc/syntax.txt

@@ -5263,6 +5263,8 @@ EndOfBuffer	Filler lines (~) after the last line in the buffer.
 		By default, this is highlighted like |hl-NonText|.
 							*hl-TermCursor*
 TermCursor	Cursor in a focused terminal.
+							*hl-OkMsg*
+OkMsg		Success messages on the command line.
 							*hl-ErrorMsg*
 ErrorMsg	Error messages on the command line.
 							*hl-StderrMsg*

src/nvim/api/vim.c

@@ -796,6 +796,7 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool
   }
 
   bool is_progress = strequal(kind, "progress");
+  bool needs_clear = !history;
 
   VALIDATE(is_progress
            || (opts->status.size == 0 && opts->title.size == 0 && opts->percent == 0
@@ -822,7 +823,7 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool
   MessageData msg_data = { .title = opts->title, .status = opts->status,
                            .percent = opts->percent, .data = opts->data };
 
-  id = msg_multihl(opts->id, hl_msg, kind, history, opts->err, &msg_data);
+  id = msg_multihl(opts->id, hl_msg, kind, history, opts->err, &msg_data, &needs_clear);
 
   if (opts->verbose) {
     verbose_leave();
@@ -833,8 +834,8 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool
     do_autocmd_progress(id, hl_msg, &msg_data);
   }
 
-  if (history) {
+  if (!needs_clear) {
-    // history takes ownership
+    // history takes ownership of `hl_msg`
     return id;
   }
 

src/nvim/highlight.h

@@ -84,6 +84,7 @@ EXTERN const char *hlf_names[] INIT( = {
   [HLF_BFOOTER] = "FloatFooter",
   [HLF_TS] = "StatusLineTerm",
   [HLF_TSNC] = "StatusLineTermNC",
+  [HLF_OK] = "OkMsg",
 });
 
 EXTERN int highlight_attr[HLF_COUNT];     // Highl. attr for each context.

src/nvim/highlight_defs.h

@@ -132,6 +132,7 @@ typedef enum {
   HLF_TSNC,       ///< status line for non-current terminal window
   HLF_SE,         ///< stderr messages (from shell)
   HLF_SO,         ///< stdout messages (from shell)
+  HLF_OK,         ///< OK message
   HLF_COUNT,      ///< MUST be the last one
 } hlf_T;
 

src/nvim/highlight_group.c

@@ -375,6 +375,7 @@ static const char *highlight_init_light[] = {
   "MoreMsg              guifg=NvimDarkCyan                                   ctermfg=6",
   "NonText              guifg=NvimLightGrey4",
   "NormalFloat                               guibg=NvimLightGrey1",
+  "OkMsg                guifg=NvimDarkGreen                                  ctermfg=2",
   "Pmenu                                     guibg=NvimLightGrey3            cterm=reverse",
   "PmenuThumb                                guibg=NvimLightGrey4",
   "Question             guifg=NvimDarkCyan                                   ctermfg=6",
@@ -459,6 +460,7 @@ static const char *highlight_init_dark[] = {
   "MoreMsg              guifg=NvimLightCyan                                 ctermfg=14",
   "NonText              guifg=NvimDarkGrey4",
   "NormalFloat                                guibg=NvimDarkGrey1",
+  "OkMsg                guifg=NvimLightGreen                                ctermfg=10",
   "Pmenu                                      guibg=NvimDarkGrey3           cterm=reverse",
   "PmenuThumb                                 guibg=NvimDarkGrey4",
   "Question             guifg=NvimLightCyan                                 ctermfg=14",

src/nvim/lua/executor.c

@@ -994,7 +994,8 @@ static void nlua_print_event(void **argv)
   HlMessage msg = KV_INITIAL_VALUE;
   HlMessageChunk chunk = { { .data = argv[0], .size = (size_t)(intptr_t)argv[1] - 1 }, 0 };
   kv_push(msg, chunk);
-  msg_multihl(INTEGER_OBJ(0), msg, "lua_print", true, false, NULL);
+  bool needs_clear = false;
+  msg_multihl(INTEGER_OBJ(0), msg, "lua_print", true, false, NULL, &needs_clear);
 }
 
 /// Print as a Vim message

src/nvim/message.c

@@ -292,14 +292,61 @@ void msg_multiline(String str, int hl_id, bool check_int, bool hist, bool *need_
 // Avoid starting a new message for each chunk and adding message to history in msg_keep().
 static bool is_multihl = false;
 
+/// Format a progress message, adding title and percent if given.
+///
+/// @param hl_msg Message chunks
+/// @param msg_data Additional data for progress messages
+static HlMessage format_progress_message(HlMessage hl_msg, MessageData *msg_data)
+{
+  HlMessage updated_msg = KV_INITIAL_VALUE;
+  // progress messages are special. displayed as "title: percent% msg"
+  if (msg_data->title.size != 0) {
+    // this block draws the "title:" before the progress-message
+    int hl_id = 0;
+    if (msg_data->status.data == NULL) {
+      hl_id = 0;
+    } else if (strequal(msg_data->status.data, "success")) {
+      hl_id = syn_check_group("OkMsg", STRLEN_LITERAL("OkMsg"));
+    } else if (strequal(msg_data->status.data, "failed")) {
+      hl_id = syn_check_group("ErrorMsg", STRLEN_LITERAL("ErrorMsg"));
+    } else if (strequal(msg_data->status.data, "running")) {
+      hl_id = syn_check_group("MoreMsg", STRLEN_LITERAL("MoreMsg"));
+    } else if (strequal(msg_data->status.data, "cancel")) {
+      hl_id = syn_check_group("WarningMsg", STRLEN_LITERAL("WarningMsg"));
+    }
+    kv_push(updated_msg,
+            ((HlMessageChunk){ .text = copy_string(msg_data->title, NULL), .hl_id = hl_id }));
+    kv_push(updated_msg, ((HlMessageChunk){ .text = cstr_to_string(": "), .hl_id = 0 }));
+  }
+  if (msg_data->percent > 0) {
+    char percent_buf[10];
+    vim_snprintf(percent_buf, sizeof(percent_buf), "%3ld%% ", (long)msg_data->percent);
+    String percent = cstr_to_string(percent_buf);
+    int hl_id = syn_check_group("WarningMsg", STRLEN_LITERAL("WarningMsg"));
+    kv_push(updated_msg, ((HlMessageChunk){ .text = percent, .hl_id = hl_id }));
+  }
+
+  if (kv_size(updated_msg) != 0) {
+    for (uint32_t i = 0; i < kv_size(hl_msg); i++) {
+      kv_push(updated_msg,
+              ((HlMessageChunk){ .text = copy_string(kv_A(hl_msg, i).text, NULL),
+                                 .hl_id = kv_A(hl_msg, i).hl_id }));
+    }
+    return updated_msg;
+  } else {
+    return hl_msg;
+  }
+}
+
 /// Print message chunks, each with their own highlight ID.
 ///
 /// @param hl_msg Message chunks
 /// @param kind Message kind (can be NULL to avoid setting kind)
 /// @param history Whether to add message to history
 /// @param err Whether to print message as an error
+/// @param msg_data Additional data for progress messages
 MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bool err,
-                  MessageData *msg_data)
+                  MessageData *msg_data, bool *needs_msg_clear)
 {
   no_wait_return++;
   msg_start();
@@ -311,7 +358,6 @@ MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bo
   }
   is_multihl = true;
   msg_ext_skip_flush = true;
-  bool is_progress = strequal(kind, "progress");
 
   // provide a new id if not given
   if (id.type == kObjectTypeNil) {
@@ -323,12 +369,13 @@ MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bo
     }
   }
 
-  // progress message are special displayed as "title: msg...percent%"
+  // progress message are special displayed as "title: percent% msg"
-  if (is_progress && msg_data && msg_data->title.size != 0) {
+  if (strequal(kind, "progress") && msg_data) {
-    // this block draws the "title:" before the progress-message
+    HlMessage formated_message = format_progress_message(hl_msg, msg_data);
-    String title = cstr_as_string(concat_str(msg_data->title.data, ": "));
+    if (formated_message.items != hl_msg.items) {
-    msg_multiline(title, 0, true, false, &need_clear);
+      *needs_msg_clear = true;
-    api_free_string(title);
+      hl_msg = formated_message;
+    }
   }
 
   for (uint32_t i = 0; i < kv_size(hl_msg); i++) {
@@ -341,12 +388,6 @@ MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bo
     assert(!ui_has(kUIMessages) || kind == NULL || msg_ext_kind == kind);
   }
 
-  if (is_progress && msg_data && msg_data->percent > 0) {
-    // this block draws the "...percent%" before the progress-message
-    char percent_buf[10];
-    vim_snprintf(percent_buf, sizeof(percent_buf), "...%ld%%", (long)msg_data->percent);
-    msg_multiline(cstr_as_string(percent_buf), 0, true, false, &need_clear);
-  }
   if (history && kv_size(hl_msg)) {
     msg_hist_add_multihl(id, hl_msg, false, msg_data);
   }
@@ -1265,7 +1306,8 @@ void ex_messages(exarg_T *eap)
     }
     if (redirecting() || !ui_has(kUIMessages)) {
       msg_silent += ui_has(kUIMessages);
-      msg_multihl(INTEGER_OBJ(0), p->msg, p->kind, false, false, NULL);
+      bool needs_clear = false;
+      msg_multihl(INTEGER_OBJ(0), p->msg, p->kind, false, false, NULL, &needs_clear);
       msg_silent -= ui_has(kUIMessages);
     }
   }

test/functional/ui/cursor_spec.lua

@@ -262,7 +262,7 @@ describe('ui/cursor', function()
         m.attr = { background = Screen.colors.DarkGray }
       end
       if m.id_lm then
-        m.id_lm = 75
+        m.id_lm = 76
         m.attr_lm = {}
       end
     end

test/functional/ui/messages_spec.lua

@@ -3145,6 +3145,7 @@ describe('progress-message', function()
   local function setup_autocmd(pattern)
     exec_lua(function()
       local grp = vim.api.nvim_create_augroup('ProgressListener', { clear = true })
+      _G.progress_autocmd_result = nil
       vim.api.nvim_create_autocmd('Progress', {
         pattern = pattern,
         group = grp,
@@ -3171,6 +3172,7 @@ describe('progress-message', function()
       screen:add_extra_attr_ids {
         [100] = { undercurl = true, special = Screen.colors.Red },
         [101] = { foreground = Screen.colors.Magenta1, bold = true },
+        [102] = { foreground = Screen.colors.NvimDarkGreen },
       }
     else
       screen = Screen.new(40, 5)
@@ -3192,19 +3194,23 @@ describe('progress-message', function()
 
     screen:expect({
       grid = [[
-        ^                         |
+    ^                         |
-        {1:~                        }|*4
+    {1:~                        }|*4
-      ]],
+  ]],
       messages = {
         {
-          content = { { 'testsuit: test-message...10%' } },
+          content = {
+            { 'testsuit', 6, 'MoreMsg' },
+            { ': ' },
+            { ' 10% ', 19, 'WarningMsg' },
+            { 'test-message' },
+          },
           history = true,
           id = 1,
           kind = 'progress',
         },
       },
     })
-
     assert_progress_autocmd({
       text = { 'test-message' },
       percent = 10,
@@ -3227,7 +3233,12 @@ describe('progress-message', function()
       ]],
       messages = {
         {
-          content = { { 'TestSuit: test-message-updated...50%' } },
+          content = {
+            { 'TestSuit', 6, 'MoreMsg' },
+            { ': ' },
+            { ' 50% ', 19, 'WarningMsg' },
+            { 'test-message-updated' },
+          },
           history = true,
           id = 1,
           kind = 'progress',
@@ -3244,6 +3255,105 @@ describe('progress-message', function()
       data = {},
     }, 'Progress autocmd receives progress update')
 
+    -- success status
+    api.nvim_echo(
+      { { 'test-message (success)' } },
+      true,
+      { kind = 'progress', title = 'TestSuit', percent = 100, status = 'success' }
+    )
+    screen:expect({
+      grid = [[
+        ^                         |
+        {1:~                        }|*4
+      ]],
+      messages = {
+        {
+          content = {
+            { 'TestSuit', 102, 'OkMsg' },
+            { ': ' },
+            { '100% ', 19, 'WarningMsg' },
+            { 'test-message (success)' },
+          },
+          history = true,
+          id = 2,
+          kind = 'progress',
+        },
+      },
+    })
+
+    -- failed status
+    api.nvim_echo(
+      { { 'test-message (success)' } },
+      true,
+      { kind = 'progress', title = 'TestSuit', percent = 35, status = 'failed' }
+    )
+    screen:expect({
+      grid = [[
+        ^                         |
+        {1:~                        }|*4
+      ]],
+      messages = {
+        {
+          content = {
+            { 'TestSuit', 9, 'ErrorMsg' },
+            { ': ' },
+            { ' 35% ', 19, 'WarningMsg' },
+            { 'test-message (success)' },
+          },
+          history = true,
+          id = 3,
+          kind = 'progress',
+        },
+      },
+    })
+
+    -- cancel status
+    api.nvim_echo(
+      { { 'test-message (success)' } },
+      true,
+      { kind = 'progress', title = 'TestSuit', percent = 30, status = 'cancel' }
+    )
+    screen:expect({
+      grid = [[
+    ^                         |
+    {1:~                        }|*4
+  ]],
+      messages = {
+        {
+          content = {
+            { 'TestSuit', 19, 'WarningMsg' },
+            { ': ' },
+            { ' 30% ', 19, 'WarningMsg' },
+            { 'test-message (success)' },
+          },
+          history = true,
+          id = 4,
+          kind = 'progress',
+        },
+      },
+    })
+
+    -- without title and percent
+    api.nvim_echo(
+      { { 'test-message (no-tile or percent)' } },
+      true,
+      { kind = 'progress', status = 'cancel' }
+    )
+    screen:expect({
+      grid = [[
+        ^                         |
+        {1:~                        }|*4
+      ]],
+      messages = {
+        {
+          content = { { 'test-message (no-tile or percent)' } },
+          history = true,
+          id = 5,
+          kind = 'progress',
+        },
+      },
+    })
+
     -- progress event can filter by title
     setup_autocmd('Special Title')
     api.nvim_echo(
@@ -3284,7 +3394,12 @@ describe('progress-message', function()
       ]],
       messages = {
         {
-          content = { { 'TestSuit: test-message...10%' } },
+          content = {
+            { 'TestSuit', 6, 'MoreMsg' },
+            { ': ' },
+            { ' 10% ', 19, 'WarningMsg' },
+            { 'test-message' },
+          },
           history = true,
           id = 1,
           kind = 'progress',
@@ -3373,23 +3488,29 @@ describe('progress-message', function()
       true,
       { kind = 'progress', title = 'TestSuit', percent = 10, status = 'running' }
     )
-    eq('test-message 10', exec_capture('messages'))
+    eq('TestSuit:  10% test-message 10', exec_capture('messages'))
 
     api.nvim_echo(
       { { 'test-message 20' } },
       true,
       { id = id, kind = 'progress', title = 'TestSuit', percent = 20, status = 'running' }
     )
-    eq('test-message 10\ntest-message 20', exec_capture('messages'))
+    eq('TestSuit:  10% test-message 10\nTestSuit:  20% test-message 20', exec_capture('messages'))
 
     api.nvim_echo({ { 'middle msg' } }, true, {})
-    eq('test-message 10\ntest-message 20\nmiddle msg', exec_capture('messages'))
+    eq(
+      'TestSuit:  10% test-message 10\nTestSuit:  20% test-message 20\nmiddle msg',
+      exec_capture('messages')
+    )
     api.nvim_echo(
       { { 'test-message 30' } },
       true,
       { id = id, kind = 'progress', title = 'TestSuit', percent = 30, status = 'running' }
     )
-    eq('test-message 10\ntest-message 20\nmiddle msg\ntest-message 30', exec_capture('messages'))
+    eq(
+      'TestSuit:  10% test-message 10\nTestSuit:  20% test-message 20\nmiddle msg\nTestSuit:  30% test-message 30',
+      exec_capture('messages')
+    )
 
     api.nvim_echo(
       { { 'test-message 50' } },
@@ -3397,7 +3518,7 @@ describe('progress-message', function()
       { id = id, kind = 'progress', title = 'TestSuit', percent = 50, status = 'running' }
     )
     eq(
-      'test-message 10\ntest-message 20\nmiddle msg\ntest-message 30\ntest-message 50',
+      'TestSuit:  10% test-message 10\nTestSuit:  20% test-message 20\nmiddle msg\nTestSuit:  30% test-message 30\nTestSuit:  50% test-message 50',
       exec_capture('messages')
     )
   end)
@@ -3476,7 +3597,12 @@ describe('progress-message', function()
       ]],
       messages = {
         {
-          content = { { 'TestSuit: supports str-id...30%' } },
+          content = {
+            { 'TestSuit', 6, 'MoreMsg' },
+            { ': ' },
+            { ' 30% ', 19, 'WarningMsg' },
+            { 'supports str-id' },
+          },
           history = true,
           id = 'str-id',
           kind = 'progress',
@@ -3511,7 +3637,7 @@ describe('progress-message', function()
     screen:expect([[
       ^                                        |
       {1:~                                       }|*3
-      TestSuit: test-message...10%            |
+      {6:TestSuit}: {19: 10% }test-message             |
     ]])
   end)
 end)