patch 9.1.0883: message history cleanup is missing some tests

Problem:  message history cleanup is missing some tests
Solution: Add tests, refactor common code into did_set_msghistory()
          (Shougo Matsushita)

closes: #16078

Co-authored-by: zeertzjq <zeertzjq@outlook.com>
Co-authored-by: Milly <milly.ca@gmail.com>
Signed-off-by: Shougo Matsushita <Shougo.Matsu@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

runtime/doc/options.txt

@@ -1,4 +1,4 @@
-*options.txt*	For Vim version 9.1.  Last change: 2024 Nov 14
+*options.txt*	For Vim version 9.1.  Last change: 2024 Nov 24
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -5923,6 +5923,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 			global
 	Determines how many entries are remembered in the |:messages| history.
 	The maximum value is 10000.
+	Setting it to zero clears the message history.
+
 
 						    *'mzquantum'* *'mzq'*
 'mzquantum' 'mzq'	number	(default 100)

src/message.c

@@ -1011,10 +1011,6 @@ add_msg_hist(
     if (msg_hist_off || msg_silent != 0)
 	return;
 
-    // Don't let the message history get too big
-    while (msg_hist_len > p_mhi)
-	(void)delete_first_msg();
-
     // allocate an entry and add the message at the end of the history
     p = ALLOC_ONE(struct msg_hist);
     if (p == NULL)
@@ -1039,6 +1035,8 @@ add_msg_hist(
     if (first_msg_hist == NULL)
 	first_msg_hist = last_msg_hist;
     ++msg_hist_len;
+
+    check_msg_hist();
 }
 
 /*
@@ -1062,6 +1060,14 @@ delete_first_msg(void)
     return OK;
 }
 
+    void
+check_msg_hist(void)
+{
+    // Don't let the message history get too big
+    while (msg_hist_len > 0 && msg_hist_len > p_mhi)
+	(void)delete_first_msg();
+}
+
 /*
  * ":messages" command.
  */

src/option.c

@@ -3864,6 +3864,31 @@ did_set_number_relativenumber(optset_T *args UNUSED)
     return NULL;
 }
 
+/*
+ * Process the updated 'msghistory' option value.
+ */
+    char *
+did_set_msghistory(optset_T *args UNUSED)
+{
+    char *errmsg = NULL;
+
+    // 'msghistory' must be positive
+    if (p_mhi < 0)
+    {
+	errmsg = e_argument_must_be_positive;
+	p_mhi = 0;
+    }
+    else if (p_mhi > 10000)
+    {
+	errmsg = e_invalid_argument;
+	p_mhi = 10000;
+    }
+
+    check_msg_hist();
+
+    return errmsg;
+}
+
 #if defined(FEAT_LINEBREAK) || defined(PROTO)
 /*
  * Process the new 'numberwidth' option value.
@@ -4914,16 +4939,6 @@ check_num_option_bounds(
 	errmsg = e_invalid_argument;
 	p_hi = 10000;
     }
-    if (p_mhi < 0)
-    {
-	errmsg = e_argument_must_be_positive;
-	p_mhi = 0;
-    }
-    else if (p_mhi > 10000)
-    {
-	errmsg = e_invalid_argument;
-	p_mhi = 10000;
-    }
     if (p_re < 0 || p_re > 2)
     {
 	errmsg = e_invalid_argument;

src/optiondefs.h

@@ -1779,7 +1779,7 @@ static struct vimoption options[] =
 			    (char_u *)&p_mouset, PV_NONE, NULL, NULL,
 			    {(char_u *)500L, (char_u *)0L} SCTX_INIT},
     {"msghistory","mhi",    P_NUM|P_VI_DEF,
-			    (char_u *)&p_mhi, PV_NONE, NULL, NULL,
+			    (char_u *)&p_mhi, PV_NONE, did_set_msghistory, NULL,
 			    {(char_u *)500L, (char_u *)0L} SCTX_INIT},
     {"mzquantum",  "mzq",   P_NUM,
 #ifdef FEAT_MZSCHEME

src/proto/message.pro

@@ -18,6 +18,7 @@ void emsg_namelen(char *msg, char_u *name, int len);
 char *msg_trunc_attr(char *s, int force, int attr);
 char_u *msg_may_trunc(int force, char_u *s);
 int delete_first_msg(void);
+void check_msg_hist(void);
 void ex_messages(exarg_T *eap);
 void msg_end_prompt(void);
 void wait_return(int redraw);

src/proto/option.pro

@@ -56,6 +56,7 @@ char *did_set_maxcombine(optset_T *args);
 char *did_set_modifiable(optset_T *args);
 char *did_set_modified(optset_T *args);
 char *did_set_mousehide(optset_T *args);
+char *did_set_msghistory(optset_T *args);
 char *did_set_number_relativenumber(optset_T *args);
 char *did_set_numberwidth(optset_T *args);
 char *did_set_paste(optset_T *args);

src/testdir/test_cmdline.vim

@@ -4011,4 +4011,30 @@ func Test_cd_bslash_completion_windows()
   let &shellslash = save_shellslash
 endfunc
 
+func Test_msghistory()
+  " After setting 'msghistory' to 2 and outputting a message 4 times with
+  " :echomsg, is the number of output lines of :messages 2?
+  set msghistory=2
+  echomsg 'foo'
+  echomsg 'bar'
+  echomsg 'baz'
+  echomsg 'foobar'
+  call assert_equal(['baz', 'foobar'], GetMessages())
+
+  " When the number of messages is 10 and 'msghistory' is changed to 5, is the
+  " number of output lines of :messages 5?
+  set msghistory=10
+  for num in range(1, 10)
+    echomsg num
+  endfor
+  set msghistory=5
+  call assert_equal(5, len(GetMessages()))
+
+  " Check empty list
+  set msghistory=0
+  call assert_true(empty(GetMessages()))
+
+  set msghistory&
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

src/version.c

@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    883,
 /**/
     882,
 /**/