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>
diff --git a/src/message.c b/src/message.c
index 95bd726..15eaaae 100644
--- a/src/message.c
+++ b/src/message.c
@@ -1011,10 +1011,6 @@
     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 @@
     if (first_msg_hist == NULL)
 	first_msg_hist = last_msg_hist;
     ++msg_hist_len;
+
+    check_msg_hist();
 }
 
 /*
@@ -1062,6 +1060,14 @@
     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.
  */
diff --git a/src/option.c b/src/option.c
index 4616d63..226c31d 100644
--- a/src/option.c
+++ b/src/option.c
@@ -3864,6 +3864,31 @@
     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 @@
 	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;
diff --git a/src/optiondefs.h b/src/optiondefs.h
index 81bb1db..5df5fb7 100644
--- a/src/optiondefs.h
+++ b/src/optiondefs.h
@@ -1779,7 +1779,7 @@
 			    (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
diff --git a/src/proto/message.pro b/src/proto/message.pro
index 54a0a77..1c11444 100644
--- a/src/proto/message.pro
+++ b/src/proto/message.pro
@@ -18,6 +18,7 @@
 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);
diff --git a/src/proto/option.pro b/src/proto/option.pro
index 83f32aa..8aa49c0 100644
--- a/src/proto/option.pro
+++ b/src/proto/option.pro
@@ -56,6 +56,7 @@
 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);
diff --git a/src/testdir/test_cmdline.vim b/src/testdir/test_cmdline.vim
index 110be65..9aec012 100644
--- a/src/testdir/test_cmdline.vim
+++ b/src/testdir/test_cmdline.vim
@@ -4011,4 +4011,30 @@
   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
diff --git a/src/version.c b/src/version.c
index 9adbf9b..6f01f78 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    883,
+/**/
     882,
 /**/
     881,