patch 9.1.0422: function echo_string_core() is too longv9.1.0422

Problem:  function echo_string_core() is too long
Solution: Refactor into several smaller functions
          (Yegappan Lakshmanan)

closes: #14804

Signed-off-by: Yegappan Lakshmanan <yegappan@yahoo.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

5 files changed, 308 insertions, 162 deletions

diff --git a/src/eval.c b/src/eval.c
index 8cc6ed1ed5..833a230f9d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -6450,10 +6450,291 @@ set_ref_in_item(
 }
 
 /*
+ * Return a textual representation of a string in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * When both "echo_style" and "composite_val" are FALSE, put quotes around
+ * strings as "string()", otherwise does not put quotes around strings.
+ * May return NULL.
+ */
+    static char_u *
+string_tv2string(
+    typval_T	*tv,
+    char_u	**tofree,
+    int		echo_style,
+    int		composite_val)
+{
+    char_u	*r = NULL;
+
+    if (echo_style && !composite_val)
+    {
+	*tofree = NULL;
+	r = tv->vval.v_string;
+	if (r == NULL)
+	    r = (char_u *)"";
+    }
+    else
+    {
+	*tofree = string_quote(tv->vval.v_string, FALSE);
+	r = *tofree;
+    }
+
+    return r;
+}
+
+/*
+ * Return a textual representation of a function in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * When "echo_style" is FALSE, put quotes around the function name as
+ * "function()", otherwise does not put quotes around function name.
+ * May return NULL.
+ */
+    static char_u *
+func_tv2string(typval_T *tv, char_u **tofree, int echo_style)
+{
+    char_u	*r = NULL;
+    char_u	buf[MAX_FUNC_NAME_LEN];
+
+    if (echo_style)
+    {
+	r = tv->vval.v_string == NULL ? (char_u *)"function()"
+				: make_ufunc_name_readable(tv->vval.v_string,
+						buf, MAX_FUNC_NAME_LEN);
+	if (r == buf)
+	{
+	    r = vim_strsave(buf);
+	    *tofree = r;
+	}
+	else
+	    *tofree = NULL;
+    }
+    else
+    {
+	*tofree = string_quote(tv->vval.v_string == NULL ? NULL
+				: make_ufunc_name_readable(tv->vval.v_string,
+					buf, MAX_FUNC_NAME_LEN), TRUE);
+	r = *tofree;
+    }
+
+    return r;
+}
+
+/*
+ * Return a textual representation of a partial in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * "numbuf" is used for a number.  May return NULL.
+ */
+    static char_u *
+partial_tv2string(
+    typval_T	*tv,
+    char_u	**tofree,
+    char_u	*numbuf,
+    int		copyID)
+{
+    char_u	*r = NULL;
+    partial_T	*pt;
+    char_u	*fname;
+    garray_T	ga;
+    int		i;
+    char_u	*tf;
+
+    pt = tv->vval.v_partial;
+    fname = string_quote(pt == NULL ? NULL : partial_name(pt), FALSE);
+
+    ga_init2(&ga, 1, 100);
+    ga_concat(&ga, (char_u *)"function(");
+    if (fname != NULL)
+    {
+	// When using uf_name prepend "g:" for a global function.
+	if (pt != NULL && pt->pt_name == NULL && fname[0] == '\''
+						&& vim_isupper(fname[1]))
+	{
+	    ga_concat(&ga, (char_u *)"'g:");
+	    ga_concat(&ga, fname + 1);
+	}
+	else
+	    ga_concat(&ga, fname);
+	vim_free(fname);
+    }
+    if (pt != NULL && pt->pt_argc > 0)
+    {
+	ga_concat(&ga, (char_u *)", [");
+	for (i = 0; i < pt->pt_argc; ++i)
+	{
+	    if (i > 0)
+		ga_concat(&ga, (char_u *)", ");
+	    ga_concat(&ga, tv2string(&pt->pt_argv[i], &tf, numbuf, copyID));
+	    vim_free(tf);
+	}
+	ga_concat(&ga, (char_u *)"]");
+    }
+    if (pt != NULL && pt->pt_dict != NULL)
+    {
+	typval_T dtv;
+
+	ga_concat(&ga, (char_u *)", ");
+	dtv.v_type = VAR_DICT;
+	dtv.vval.v_dict = pt->pt_dict;
+	ga_concat(&ga, tv2string(&dtv, &tf, numbuf, copyID));
+	vim_free(tf);
+    }
+    // terminate with ')' and a NUL
+    ga_concat_len(&ga, (char_u *)")", 2);
+
+    *tofree = ga.ga_data;
+    r = *tofree;
+
+    return r;
+}
+
+/*
+ * Return a textual representation of a List in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * When "copyID" is not zero replace recursive lists with "...".  When
+ * "restore_copyID" is FALSE, repeated items in lists are replaced with "...".
+ * May return NULL.
+ */
+    static char_u *
+list_tv2string(
+    typval_T	*tv,
+    char_u	**tofree,
+    int		copyID,
+    int		restore_copyID)
+{
+    char_u	*r = NULL;
+
+    if (tv->vval.v_list == NULL)
+    {
+	// NULL list is equivalent to empty list.
+	*tofree = NULL;
+	r = (char_u *)"[]";
+    }
+    else if (copyID != 0 && tv->vval.v_list->lv_copyID == copyID
+	    && tv->vval.v_list->lv_len > 0)
+    {
+	*tofree = NULL;
+	r = (char_u *)"[...]";
+    }
+    else
+    {
+	int old_copyID = tv->vval.v_list->lv_copyID;
+
+	tv->vval.v_list->lv_copyID = copyID;
+	*tofree = list2string(tv, copyID, restore_copyID);
+	if (restore_copyID)
+	    tv->vval.v_list->lv_copyID = old_copyID;
+	r = *tofree;
+    }
+
+    return r;
+}
+
+/*
+ * Return a textual representation of a Dict in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * When "copyID" is not zero replace recursive dicts with "...".
+ * When "restore_copyID" is FALSE, repeated items in the dictionary are
+ * replaced with "...".  May return NULL.
+ */
+    static char_u *
+dict_tv2string(
+    typval_T	*tv,
+    char_u	**tofree,
+    int		copyID,
+    int		restore_copyID)
+{
+    char_u	*r = NULL;
+
+    if (tv->vval.v_dict == NULL)
+    {
+	// NULL dict is equivalent to empty dict.
+	*tofree = NULL;
+	r = (char_u *)"{}";
+    }
+    else if (copyID != 0 && tv->vval.v_dict->dv_copyID == copyID
+	    && tv->vval.v_dict->dv_hashtab.ht_used != 0)
+    {
+	*tofree = NULL;
+	r = (char_u *)"{...}";
+    }
+    else
+    {
+	int old_copyID = tv->vval.v_dict->dv_copyID;
+
+	tv->vval.v_dict->dv_copyID = copyID;
+	*tofree = dict2string(tv, copyID, restore_copyID);
+	if (restore_copyID)
+	    tv->vval.v_dict->dv_copyID = old_copyID;
+	r = *tofree;
+    }
+
+    return r;
+}
+
+/*
+ * Return a textual representation of a job or a channel in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * "numbuf" is used for a number.
+ * When "composite_val" is FALSE, put quotes around strings as "string()",
+ * otherwise does not put quotes around strings.
+ * May return NULL.
+ */
+    static char_u *
+jobchan_tv2string(
+    typval_T	*tv,
+    char_u	**tofree,
+    char_u	*numbuf,
+    int		composite_val)
+{
+    char_u	*r = NULL;
+
+#ifdef FEAT_JOB_CHANNEL
+    *tofree = NULL;
+
+    if (tv->v_type == VAR_JOB)
+	r = job_to_string_buf(tv, numbuf);
+    else
+	r = channel_to_string_buf(tv, numbuf);
+
+    if (composite_val)
+    {
+	*tofree = string_quote(r, FALSE);
+	r = *tofree;
+    }
+#endif
+
+    return r;
+}
+
+/*
+ * Return a textual representation of a class in "tv".
+ * If the memory is allocated "tofree" is set to it, otherwise NULL.
+ * May return NULL.
+ */
+    static char_u *
+class_tv2string(typval_T *tv, char_u **tofree)
+{
+    char_u	*r = NULL;
+    class_T	*cl = tv->vval.v_class;
+    char	*s = "class";
+
+    if (cl != NULL && IS_INTERFACE(cl))
+	s = "interface";
+    else if (cl != NULL && IS_ENUM(cl))
+	s = "enum";
+    size_t len = STRLEN(s) + 1 +
+				(cl == NULL ? 9 : STRLEN(cl->class_name)) + 1;
+    r = *tofree = alloc(len);
+    vim_snprintf((char *)r, len, "%s %s", s,
+			cl == NULL ? "[unknown]" : (char *)cl->class_name);
+
+    return r;
+}
+
+/*
  * Return a string with the string representation of a variable.
  * If the memory is allocated "tofree" is set to it, otherwise NULL.
  * "numbuf" is used for a number.
- * When "copyID" is not NULL replace recursive lists and dicts with "...".
+ * When "copyID" is not zero replace recursive lists and dicts with "...".
  * When both "echo_style" and "composite_val" are FALSE, put quotes around
  * strings as "string()", otherwise does not put quotes around strings, as
  * ":echo" displays values.
@@ -6492,155 +6773,27 @@ echo_string_core(
     switch (tv->v_type)
     {
 	case VAR_STRING:
-	    if (echo_style && !composite_val)
-	    {
-		*tofree = NULL;
-		r = tv->vval.v_string;
-		if (r == NULL)
-		    r = (char_u *)"";
-	    }
-	    else
-	    {
-		*tofree = string_quote(tv->vval.v_string, FALSE);
-		r = *tofree;
-	    }
+	    r = string_tv2string(tv, tofree, echo_style, composite_val);
 	    break;
 
 	case VAR_FUNC:
-	    {
-		char_u buf[MAX_FUNC_NAME_LEN];
-
-		if (echo_style)
-		{
-		    r = tv->vval.v_string == NULL ? (char_u *)"function()"
-				  : make_ufunc_name_readable(tv->vval.v_string,
-						       buf, MAX_FUNC_NAME_LEN);
-		    if (r == buf)
-		    {
-			r = vim_strsave(buf);
-			*tofree = r;
-		    }
-		    else
-			*tofree = NULL;
-		}
-		else
-		{
-		    *tofree = string_quote(tv->vval.v_string == NULL ? NULL
-			    : make_ufunc_name_readable(
-				tv->vval.v_string, buf, MAX_FUNC_NAME_LEN),
-									 TRUE);
-		    r = *tofree;
-		}
-	    }
+	    r = func_tv2string(tv, tofree, echo_style);
 	    break;
 
 	case VAR_PARTIAL:
-	    {
-		partial_T   *pt = tv->vval.v_partial;
-		char_u	    *fname = string_quote(pt == NULL ? NULL
-						    : partial_name(pt), FALSE);
-		garray_T    ga;
-		int	    i;
-		char_u	    *tf;
-
-		ga_init2(&ga, 1, 100);
-		ga_concat(&ga, (char_u *)"function(");
-		if (fname != NULL)
-		{
-		    // When using uf_name prepend "g:" for a global function.
-		    if (pt != NULL && pt->pt_name == NULL && fname[0] == '\''
-						      && vim_isupper(fname[1]))
-		    {
-			ga_concat(&ga, (char_u *)"'g:");
-			ga_concat(&ga, fname + 1);
-		    }
-		    else
-			ga_concat(&ga, fname);
-		    vim_free(fname);
-		}
-		if (pt != NULL && pt->pt_argc > 0)
-		{
-		    ga_concat(&ga, (char_u *)", [");
-		    for (i = 0; i < pt->pt_argc; ++i)
-		    {
-			if (i > 0)
-			    ga_concat(&ga, (char_u *)", ");
-			ga_concat(&ga,
-			     tv2string(&pt->pt_argv[i], &tf, numbuf, copyID));
-			vim_free(tf);
-		    }
-		    ga_concat(&ga, (char_u *)"]");
-		}
-		if (pt != NULL && pt->pt_dict != NULL)
-		{
-		    typval_T dtv;
-
-		    ga_concat(&ga, (char_u *)", ");
-		    dtv.v_type = VAR_DICT;
-		    dtv.vval.v_dict = pt->pt_dict;
-		    ga_concat(&ga, tv2string(&dtv, &tf, numbuf, copyID));
-		    vim_free(tf);
-		}
-		// terminate with ')' and a NUL
-		ga_concat_len(&ga, (char_u *)")", 2);
-
-		*tofree = ga.ga_data;
-		r = *tofree;
-		break;
-	    }
+	    r = partial_tv2string(tv, tofree, numbuf, copyID);
+	    break;
 
 	case VAR_BLOB:
 	    r = blob2string(tv->vval.v_blob, tofree, numbuf);
 	    break;
 
 	case VAR_LIST:
-	    if (tv->vval.v_list == NULL)
-	    {
-		// NULL list is equivalent to empty list.
-		*tofree = NULL;
-		r = (char_u *)"[]";
-	    }
-	    else if (copyID != 0 && tv->vval.v_list->lv_copyID == copyID
-		    && tv->vval.v_list->lv_len > 0)
-	    {
-		*tofree = NULL;
-		r = (char_u *)"[...]";
-	    }
-	    else
-	    {
-		int old_copyID = tv->vval.v_list->lv_copyID;
-
-		tv->vval.v_list->lv_copyID = copyID;
-		*tofree = list2string(tv, copyID, restore_copyID);
-		if (restore_copyID)
-		    tv->vval.v_list->lv_copyID = old_copyID;
-		r = *tofree;
-	    }
+	    r = list_tv2string(tv, tofree, copyID, restore_copyID);
 	    break;
 
 	case VAR_DICT:
-	    if (tv->vval.v_dict == NULL)
-	    {
-		// NULL dict is equivalent to empty dict.
-		*tofree = NULL;
-		r = (char_u *)"{}";
-	    }
-	    else if (copyID != 0 && tv->vval.v_dict->dv_copyID == copyID
-		    && tv->vval.v_dict->dv_hashtab.ht_used != 0)
-	    {
-		*tofree = NULL;
-		r = (char_u *)"{...}";
-	    }
-	    else
-	    {
-		int old_copyID = tv->vval.v_dict->dv_copyID;
-
-		tv->vval.v_dict->dv_copyID = copyID;
-		*tofree = dict2string(tv, copyID, restore_copyID);
-		if (restore_copyID)
-		    tv->vval.v_dict->dv_copyID = old_copyID;
-		r = *tofree;
-	    }
+	    r = dict_tv2string(tv, tofree, copyID, restore_copyID);
 	    break;
 
 	case VAR_NUMBER:
@@ -6653,16 +6806,7 @@ echo_string_core(
 
 	case VAR_JOB:
 	case VAR_CHANNEL:
-#ifdef FEAT_JOB_CHANNEL
-	    *tofree = NULL;
-	    r = tv->v_type == VAR_JOB ? job_to_string_buf(tv, numbuf)
-					   : channel_to_string_buf(tv, numbuf);
-	    if (composite_val)
-	    {
-		*tofree = string_quote(r, FALSE);
-		r = *tofree;
-	    }
-#endif
+	    r = jobchan_tv2string(tv, tofree, numbuf, composite_val);
 	    break;
 
 	case VAR_INSTR:
@@ -6671,23 +6815,11 @@ echo_string_core(
 	    break;
 
 	case VAR_CLASS:
-	    {
-		class_T *cl = tv->vval.v_class;
-		char *s = "class";
-		if (cl != NULL && IS_INTERFACE(cl))
-		    s = "interface";
-		else if (cl != NULL && IS_ENUM(cl))
-		    s = "enum";
-		size_t len = STRLEN(s) + 1 +
-		    (cl == NULL ? 9 : STRLEN(cl->class_name)) + 1;
-		r = *tofree = alloc(len);
-		vim_snprintf((char *)r, len, "%s %s", s,
-			    cl == NULL ? "[unknown]" : (char *)cl->class_name);
-	    }
+	    r = class_tv2string(tv, tofree);
 	    break;
 
 	case VAR_OBJECT:
-	    *tofree = r = object_string(tv->vval.v_object, numbuf, copyID,
+	    *tofree = r = object2string(tv->vval.v_object, numbuf, copyID,
 					echo_style, restore_copyID,
 					composite_val);
 	    break;
@@ -6722,7 +6854,7 @@ echo_string_core(
  * If the memory is allocated "tofree" is set to it, otherwise NULL.
  * "numbuf" is used for a number.
  * Does not put quotes around strings, as ":echo" displays values.
- * When "copyID" is not NULL replace recursive lists and dicts with "...".
+ * When "copyID" is not zero replace recursive lists and dicts with "...".
  * May return NULL.
  */
     char_u *
diff --git a/src/proto/vim9class.pro b/src/proto/vim9class.pro
index d3d3b99be3..1b5800cc58 100644
--- a/src/proto/vim9class.pro
+++ b/src/proto/vim9class.pro
@@ -40,7 +40,7 @@ int is_class_name(char_u *name, typval_T *rettv);
 void protected_method_access_errmsg(char_u *method_name);
 int object_empty(object_T *obj);
 int object_len(object_T *obj);
-char_u *object_string(object_T *obj, char_u *numbuf, int copyID, int echo_style, int restore_copyID, int composite_val);
+char_u *object2string(object_T *obj, char_u *numbuf, int copyID, int echo_style, int restore_copyID, int composite_val);
 int class_instance_of(class_T *cl, class_T *other_cl);
 void f_instanceof(typval_T *argvars, typval_T *rettv);
 /* vim: set ft=c : */
diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index a640fce9bd..0b17150094 100644
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -2159,6 +2159,18 @@ def Test_echo_cmd()
   assert_match('^two$', g:Screenline(&lines))
 
   v9.CheckDefFailure(['echo "xxx"# comment'], 'E488:')
+
+  # Test for echoing a script local function name
+  var lines =<< trim END
+    vim9script
+    def ScriptLocalEcho()
+    enddef
+    echo ScriptLocalEcho
+  END
+  new
+  setline(1, lines)
+  assert_match('<SNR>\d\+_ScriptLocalEcho', execute('source')->split("\n")[0])
+  bw!
 enddef
 
 def Test_echomsg_cmd()
diff --git a/src/version.c b/src/version.c
index df50a43032..ce9627b449 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    422,
+/**/
     421,
 /**/
     420,
diff --git a/src/vim9class.c b/src/vim9class.c
index 21945fec34..f66aa68ec7 100644
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -3845,7 +3845,7 @@ object_len(object_T *obj)
  * Return a textual representation of object "obj"
  */
     char_u *
-object_string(
+object2string(
     object_T	*obj,
     char_u	*numbuf,
     int		copyID,