patch 9.1.1138: cmdline completion for :hi is too simplistic

Problem:  Existing cmdline completion for :highlight was barebone and
          only completed the highlight group names.

Solution: Implement full completion for the highlight group arguments
          such as guifg and cterm. If the user tries to complete
          immediately after the '=' (e.g. `hi Normal guifg=<Tab>`), the
          completion will fill in the existing value, similar to how
          cmdline completion for options work (Yee Cheng Chin).

closes: #16712

Signed-off-by: Yee Cheng Chin <ychin.git@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
diff --git a/src/cmdexpand.c b/src/cmdexpand.c
index f707b1d..c14eee2 100644
--- a/src/cmdexpand.c
+++ b/src/cmdexpand.c
@@ -3221,6 +3221,8 @@
 	ret = ExpandMappings(pat, &regmatch, numMatches, matches);
     else if (xp->xp_context == EXPAND_ARGOPT)
 	ret = expand_argopt(pat, xp, &regmatch, matches, numMatches);
+    else if (xp->xp_context == EXPAND_HIGHLIGHT_GROUP)
+	ret = expand_highlight_group(pat, xp, &regmatch, matches, numMatches);
 #if defined(FEAT_TERMINAL)
     else if (xp->xp_context == EXPAND_TERMINALOPT)
 	ret = expand_terminal_opt(pat, xp, &regmatch, matches, numMatches);
@@ -3239,18 +3241,6 @@
     return ret;
 }
 
-/*
- * Expand a list of names.
- *
- * Generic function for command line completion.  It calls a function to
- * obtain strings, one by one.	The strings are matched against a regexp
- * program.  Matching strings are copied into an array, which is returned.
- *
- * If 'fuzzy' is TRUE, then fuzzy matching is used. Otherwise, regex matching
- * is used.
- *
- * Returns OK when no problems encountered, FAIL for error (out of memory).
- */
     int
 ExpandGeneric(
     char_u	*pat,
@@ -3262,6 +3252,38 @@
 					  // returns a string from the list
     int		escaped)
 {
+    return ExpandGenericExt(
+	pat, xp, regmatch, matches, numMatches, func, escaped, 0);
+}
+
+/*
+ * Expand a list of names.
+ *
+ * Generic function for command line completion.  It calls a function to
+ * obtain strings, one by one.	The strings are matched against a regexp
+ * program.  Matching strings are copied into an array, which is returned.
+ *
+ * If 'fuzzy' is TRUE, then fuzzy matching is used. Otherwise, regex matching
+ * is used.
+ *
+ * 'sortStartIdx' allows the caller to control sorting behavior. Items before
+ * the index will not be sorted. Pass 0 to sort all, and -1 to prevent any
+ * sorting.
+ *
+ * Returns OK when no problems encountered, FAIL for error (out of memory).
+ */
+    int
+ExpandGenericExt(
+    char_u	*pat,
+    expand_T	*xp,
+    regmatch_T	*regmatch,
+    char_u	***matches,
+    int		*numMatches,
+    char_u	*((*func)(expand_T *, int)),
+					  // returns a string from the list
+    int		escaped,
+    int		sortStartIdx)
+{
     int		i;
     garray_T	ga;
     char_u	*str;
@@ -3271,6 +3293,7 @@
     int		match;
     int		sort_matches = FALSE;
     int		funcsort = FALSE;
+    int		sortStartMatchIdx = -1;
 
     fuzzy = cmdline_fuzzy_complete(pat);
     *matches = NULL;
@@ -3346,6 +3369,12 @@
 	}
 #endif
 
+	if (sortStartIdx >= 0 && i >= sortStartIdx && sortStartMatchIdx == -1)
+	{
+	    // Found first item to start sorting from. This is usually 0.
+	    sortStartMatchIdx = ga.ga_len;
+	}
+
 	++ga.ga_len;
     }
 
@@ -3371,14 +3400,14 @@
 	funcsort = TRUE;
 
     // Sort the matches.
-    if (sort_matches)
+    if (sort_matches && sortStartMatchIdx != -1)
     {
 	if (funcsort)
 	    // <SNR> functions should be sorted to the end.
 	    qsort((void *)ga.ga_data, (size_t)ga.ga_len, sizeof(char_u *),
 							   sort_func_compare);
 	else
-	    sort_strings((char_u **)ga.ga_data, ga.ga_len);
+	    sort_strings((char_u **)ga.ga_data + sortStartMatchIdx, ga.ga_len - sortStartMatchIdx);
     }
 
     if (!fuzzy)
diff --git a/src/highlight.c b/src/highlight.c
index 8c1ad80..2d96566 100644
--- a/src/highlight.c
+++ b/src/highlight.c
@@ -1691,6 +1691,8 @@
 		break;
 	    }
 
+	    // Note: Keep this in sync with get_highlight_group_key.
+
 	    // Isolate the key ("term", "ctermfg", "ctermbg", "font", "guifg"
 	    // or "guibg").
 	    while (*linep && !VIM_ISWHITE(*linep) && *linep != '=')
@@ -3058,6 +3060,7 @@
     if (message_filtered(sgp->sg_name))
 	return;
 
+    // Note: Keep this in sync with expand_highlight_group().
     didh = highlight_list_arg(id, didh, LIST_ATTR,
 				    sgp->sg_term, NULL, "term");
     didh = highlight_list_arg(id, didh, LIST_STRING,
@@ -3108,37 +3111,24 @@
 #endif
 }
 
-    static int
-highlight_list_arg(
-    int		id,
-    int		didh,
+    static char_u*
+highlight_arg_to_string(
     int		type,
     int		iarg,
     char_u	*sarg,
-    char	*name)
+    char_u	*buf)
 {
-    char_u	buf[MAX_ATTR_LEN];
-    char_u	*ts;
-    int		i;
-
-    if (got_int)
-	return FALSE;
-
-    if (type == LIST_STRING ? (sarg == NULL) : (iarg == 0))
-	return didh;
-
-    ts = buf;
     if (type == LIST_INT)
 	sprintf((char *)buf, "%d", iarg - 1);
     else if (type == LIST_STRING)
-	ts = sarg;
+	return sarg;
     else // type == LIST_ATTR
     {
 	size_t buflen;
 
 	buf[0] = NUL;
 	buflen = 0;
-	for (i = 0; i < (int)ARRAY_LENGTH(highlight_index_tab); ++i)
+	for (int i = 0; i < (int)ARRAY_LENGTH(highlight_index_tab); ++i)
 	{
 	    if (iarg & highlight_index_tab[i]->key)
 	    {
@@ -3153,6 +3143,28 @@
 	    }
 	}
     }
+    return buf;
+}
+
+    static int
+highlight_list_arg(
+    int		id,
+    int		didh,
+    int		type,
+    int		iarg,
+    char_u	*sarg,
+    char	*name)
+{
+    char_u	buf[MAX_ATTR_LEN];
+    char_u	*ts;
+
+    if (got_int)
+	return FALSE;
+
+    if (type == LIST_STRING ? (sarg == NULL) : (iarg == 0))
+	return didh;
+
+    ts = highlight_arg_to_string(type, iarg, sarg, buf);
 
     (void)syn_list_header(didh,
 	    (int)(vim_strsize(ts) + STRLEN(name) + 1), id);
@@ -4078,6 +4090,15 @@
 static void highlight_list(void);
 static void highlight_list_two(int cnt, int attr);
 
+// context for :highlight <group> <arg> expansion
+static int expand_hi_synid = 0;	    // ID for highlight group being completed
+static int expand_hi_equal_col = 0; // column where the '=' is
+static int expand_hi_include_orig = 0;	    // whether to fill the existing current value or not
+static char_u *expand_hi_curvalue = NULL;   // the existing current value
+#if defined(FEAT_EVAL) && (defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS))
+static dict_iterator_T expand_colornames_iter;	// iterator for looping through v:colornames
+#endif
+
 /*
  * Handle command line completion for :highlight command.
  */
@@ -4085,10 +4106,12 @@
 set_context_in_highlight_cmd(expand_T *xp, char_u *arg)
 {
     char_u	*p;
+    int		expand_group = TRUE;
 
     // Default: expand group names
     xp->xp_context = EXPAND_HIGHLIGHT;
     xp->xp_pattern = arg;
+    include_none = 0;
     include_link = 2;
     include_default = 1;
 
@@ -4114,9 +4137,11 @@
     // past group name
     include_link = 0;
     if (arg[1] == 'i' && arg[0] == 'N')
+    {
 	highlight_list();
-    if (STRNCMP("link", arg, p - arg) == 0
-	    || STRNCMP("clear", arg, p - arg) == 0)
+	expand_group = FALSE;
+    }
+    if (STRNCMP("link", arg, p - arg) == 0)
     {
 	xp->xp_pattern = skipwhite(p);
 	p = skiptowhite(xp->xp_pattern);
@@ -4124,10 +4149,67 @@
 	{
 	    xp->xp_pattern = skipwhite(p);
 	    p = skiptowhite(xp->xp_pattern);
+	    include_none = 1;
 	}
+	expand_group = FALSE;
+    }
+    else if (STRNCMP("clear", arg, p - arg) == 0)
+    {
+	xp->xp_pattern = skipwhite(p);
+	p = skiptowhite(xp->xp_pattern);
+	expand_group = FALSE;
     }
     if (*p != NUL)			// past group name(s)
-	xp->xp_context = EXPAND_NOTHING;
+    {
+	if (expand_group)
+	{
+	    // expansion will be done in expand_highlight_group()
+	    xp->xp_context = EXPAND_HIGHLIGHT_GROUP;
+
+	    expand_hi_synid = syn_namen2id(arg, (int)(p - arg));
+
+	    while (*p != NUL)
+	    {
+		arg = skipwhite(p);
+		p = skiptowhite(arg);
+	    }
+
+	    p = vim_strchr(arg, '=');
+	    if (p == NULL)
+	    {
+		// Didn't find a key=<value> pattern
+		xp->xp_pattern = arg;
+		expand_hi_equal_col = -1;
+		expand_hi_include_orig = FALSE;
+	    }
+	    else
+	    {
+		// Found key=<value> pattern, record the exact location
+		expand_hi_equal_col = (int)(p - xp->xp_line);
+
+		// Only include the original value if the pattern is empty
+		if (*(p + 1) == NUL)
+		    expand_hi_include_orig = TRUE;
+		else
+		    expand_hi_include_orig = FALSE;
+
+		// Account for comma-separated values
+		if (STRNCMP(arg, "term=", 5) == 0 ||
+			STRNCMP(arg, "cterm=", 6) == 0 ||
+			STRNCMP(arg, "gui=", 4) == 0)
+		{
+		    char_u *comma = vim_strrchr(p + 1, ',');
+		    if (comma != NULL)
+			p = comma;
+		}
+		xp->xp_pattern = p + 1;
+	    }
+	}
+	else
+	{
+	    xp->xp_context = EXPAND_NOTHING;
+	}
+    }
 }
 
 /*
@@ -4178,7 +4260,7 @@
 	return (char_u *)"";
 
     if (idx == highlight_ga.ga_len && include_none != 0)
-	return (char_u *)"none";
+	return (char_u *)"NONE";
     if (idx == highlight_ga.ga_len + include_none && include_default != 0)
 	return (char_u *)"default";
     if (idx == highlight_ga.ga_len + include_none + include_default
@@ -4192,6 +4274,300 @@
     return HL_TABLE()[idx].sg_name;
 }
 
+    static char_u *
+get_highlight_attr_name(expand_T *xp UNUSED, int idx)
+{
+    if (idx == 0)
+    {
+	// Fill with current value first
+	if (expand_hi_curvalue != NULL)
+	    return expand_hi_curvalue;
+	else
+	    return (char_u*)"";
+    }
+    if (idx < (int)ARRAY_LENGTH(highlight_index_tab) + 1)
+    {
+	char_u *value = highlight_index_tab[idx-1]->value.string;
+	if (expand_hi_curvalue != NULL && STRCMP(expand_hi_curvalue, value) == 0)
+	{
+	    // Already returned the current value above, just skip.
+	    return (char_u*)"";
+	}
+	return value;
+    }
+    return NULL;
+}
+
+    static char_u *
+get_highlight_cterm_color(expand_T *xp UNUSED, int idx)
+{
+    if (idx == 0)
+    {
+	// Fill with current value first
+	if (expand_hi_curvalue != NULL)
+	    return expand_hi_curvalue;
+	else
+	    return (char_u*)"";
+    }
+    // See highlight_set_cterm_color()
+    else if (idx == 1)
+	return (char_u*)"fg";
+    else if (idx == 2)
+	return (char_u*)"bg";
+    if (idx < (int)ARRAY_LENGTH(color_name_tab) + 3)
+    {
+	char_u *value = color_name_tab[idx-3].value.string;
+	return value;
+    }
+    return NULL;
+}
+
+#if defined(FEAT_EVAL) && (defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS))
+    static char_u *
+get_highlight_gui_color(expand_T *xp UNUSED, int idx)
+{
+    if (idx == 0)
+    {
+	// Fill with current value first
+	if (expand_hi_curvalue != NULL)
+	    return expand_hi_curvalue;
+	else
+	    return (char_u*)"";
+    }
+    // See color_name2handle()
+    else if (idx == 1)
+	return (char_u*)"fg";
+    else if (idx == 2)
+	return (char_u*)"bg";
+    else if (idx == 3)
+	return (char_u*)"NONE";
+
+    // Complete from v:colornames. Don't do platform specific names for now.
+    typval_T *tv_result;
+    char_u *colorname = dict_iterate_next(&expand_colornames_iter, &tv_result);
+    if (colorname != NULL)
+    {
+	// :hi command doesn't allow space, so don't suggest any malformed items
+	if (vim_strchr(colorname, ' ') != NULL)
+	    return (char_u*)"";
+
+	if (expand_hi_curvalue != NULL && STRICMP(expand_hi_curvalue, colorname) == 0)
+	{
+	    // Already returned the current value above, just skip.
+	    return (char_u*)"";
+	}
+    }
+    return colorname;
+}
+#endif
+
+    static char_u *
+get_highlight_group_key(expand_T *xp UNUSED, int idx)
+{
+    // Note: Keep this in sync with do_highlight.
+    static char *(p_hi_group_key_values[]) =
+    {
+	"term=",
+	"start=",
+	"stop=",
+	"cterm=",
+	"ctermfg=",
+	"ctermbg=",
+	"ctermul=",
+	"ctermfont=",
+#if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
+	"gui=",
+	"guifg=",
+	"guibg=",
+	"guisp=",
+#endif
+#ifdef FEAT_GUI
+	"font=",
+#endif
+	"NONE",
+    };
+
+    if (idx < (int)ARRAY_LENGTH(p_hi_group_key_values))
+	return (char_u*)p_hi_group_key_values[idx];
+    return NULL;
+}
+
+/*
+ * Command-line expansion for :hi {group-name} <args>...
+ */
+    int
+expand_highlight_group(
+	char_u	    *pat,
+	expand_T    *xp,
+	regmatch_T  *rmp,
+	char_u	    ***matches,
+	int	    *numMatches)
+{
+    if (expand_hi_equal_col != -1)
+    {
+	// List the values. First fill in the current value, then if possible colors
+	// or attribute names.
+	char_u	    *(*expandfunc)(expand_T *, int) = NULL;
+	int	    type = 0;
+	hl_group_T  *sgp = NULL;
+	int	    iarg = 0;
+	char_u	    *sarg = NULL;
+
+	int	    unsortedItems = -1; // don't sort by default
+
+	if (expand_hi_synid != 0)
+	    sgp = &HL_TABLE()[expand_hi_synid - 1]; // index is ID minus one
+
+	// Note: Keep this in sync with highlight_list_one().
+	char_u	    *name_end = xp->xp_line + expand_hi_equal_col;
+	if (name_end - xp->xp_line >= 5
+		&& STRNCMP(name_end - 5, " term", 5) == 0)
+	{
+	    expandfunc = get_highlight_attr_name;
+	    if (sgp)
+	    {
+		type = LIST_ATTR;
+		iarg = sgp->sg_term;
+	    }
+	}
+	else if (name_end - xp->xp_line >= 6
+		&& STRNCMP(name_end - 6, " cterm", 6) == 0)
+	{
+	    expandfunc = get_highlight_attr_name;
+	    if (sgp)
+	    {
+		type = LIST_ATTR;
+		iarg = sgp->sg_cterm;
+	    }
+	}
+#if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
+	else if (name_end - xp->xp_line >= 4
+		&& STRNCMP(name_end - 4, " gui", 4) == 0)
+	{
+	    expandfunc = get_highlight_attr_name;
+	    if (sgp)
+	    {
+		type = LIST_ATTR;
+		iarg = sgp->sg_gui;
+	    }
+	}
+#endif
+	else if (name_end - xp->xp_line >= 8
+		&& STRNCMP(name_end - 8, " ctermfg", 8) == 0)
+	{
+	    expandfunc = get_highlight_cterm_color;
+	    if (sgp)
+	    {
+		type = LIST_INT;
+		iarg = sgp->sg_cterm_fg;
+	    }
+	}
+	else if (name_end - xp->xp_line >= 8
+		&& STRNCMP(name_end - 8, " ctermbg", 8) == 0)
+	{
+	    expandfunc = get_highlight_cterm_color;
+	    if (sgp)
+	    {
+		type = LIST_INT;
+		iarg = sgp->sg_cterm_bg;
+	    }
+	}
+	else if (name_end - xp->xp_line >= 8
+		&& STRNCMP(name_end - 8, " ctermul", 8) == 0)
+	{
+	    expandfunc = get_highlight_cterm_color;
+	    if (sgp)
+	    {
+		type = LIST_INT;
+		iarg = sgp->sg_cterm_ul;
+	    }
+	}
+#if defined(FEAT_EVAL) && (defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS))
+	else if (name_end - xp->xp_line >= 6
+		&& STRNCMP(name_end - 6, " guifg", 6) == 0)
+	{
+	    expandfunc = get_highlight_gui_color;
+	    if (sgp)
+	    {
+		type = LIST_STRING;
+		sarg = sgp->sg_gui_fg_name;
+	    }
+	}
+	else if (name_end - xp->xp_line >= 6
+		&& STRNCMP(name_end - 6, " guibg", 6) == 0)
+	{
+	    expandfunc = get_highlight_gui_color;
+	    if (sgp)
+	    {
+		type = LIST_STRING;
+		sarg = sgp->sg_gui_bg_name;
+	    }
+	}
+	else if (name_end - xp->xp_line >= 6
+		&& STRNCMP(name_end - 6, " guisp", 6) == 0)
+	{
+	    expandfunc = get_highlight_gui_color;
+	    if (sgp)
+	    {
+		type = LIST_STRING;
+		sarg = sgp->sg_gui_sp_name;
+	    }
+	}
+#endif
+
+#if defined(FEAT_EVAL) && (defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS))
+	if (expandfunc == get_highlight_gui_color)
+	{
+	    // Top 4 items are special, after that sort all the color names
+	    unsortedItems = 4;
+
+	    dict_T *colornames_table = get_vim_var_dict(VV_COLORNAMES);
+	    typval_T colornames_val;
+	    colornames_val.v_type = VAR_DICT;
+	    colornames_val.vval.v_dict = colornames_table;
+	    dict_iterate_start(&colornames_val, &expand_colornames_iter);
+	}
+#endif
+
+	char_u	    buf[MAX_ATTR_LEN];
+
+	if (expand_hi_synid != 0 && type != 0 && expand_hi_include_orig)
+	{
+	    // Retrieve the current value to go first in completion
+	    expand_hi_curvalue = highlight_arg_to_string(
+		    type, iarg, sarg, buf);
+	}
+	else
+	    expand_hi_curvalue = NULL;
+
+	if (expandfunc != NULL)
+	{
+	    return ExpandGenericExt(
+		    pat,
+		    xp,
+		    rmp,
+		    matches,
+		    numMatches,
+		    expandfunc,
+		    FALSE,
+		    unsortedItems);
+	}
+
+	return FAIL;
+    }
+
+    // List all the key names
+    return ExpandGenericExt(
+	    pat,
+	    xp,
+	    rmp,
+	    matches,
+	    numMatches,
+	    get_highlight_group_key,
+	    FALSE,
+	    -1);
+}
+
 #if defined(FEAT_GUI) || defined(PROTO)
 /*
  * Free all the highlight group fonts.
diff --git a/src/proto/cmdexpand.pro b/src/proto/cmdexpand.pro
index dfa6d1b..bfcc5c8 100644
--- a/src/proto/cmdexpand.pro
+++ b/src/proto/cmdexpand.pro
@@ -17,6 +17,7 @@
 void set_cmd_context(expand_T *xp, char_u *str, int len, int col, int use_ccline);
 int expand_cmdline(expand_T *xp, char_u *str, int col, int *matchcount, char_u ***matches);
 int ExpandGeneric(char_u *pat, expand_T *xp, regmatch_T *regmatch, char_u ***matches, int *numMatches, char_u *((*func)(expand_T *, int)), int escaped);
+int ExpandGenericExt(char_u *pat, expand_T *xp, regmatch_T *regmatch, char_u ***matches, int *numMatches, char_u *((*func)(expand_T *, int)), int escaped, int sortStartIdx);
 void globpath(char_u *path, char_u *file, garray_T *ga, int expand_options, int dirs);
 int wildmenu_translate_key(cmdline_info_T *cclp, int key, expand_T *xp, int did_wild_list);
 int wildmenu_process_key(cmdline_info_T *cclp, int key, expand_T *xp);
diff --git a/src/proto/highlight.pro b/src/proto/highlight.pro
index 33f90c6..5318258 100644
--- a/src/proto/highlight.pro
+++ b/src/proto/highlight.pro
@@ -43,6 +43,7 @@
 void set_context_in_highlight_cmd(expand_T *xp, char_u *arg);
 char_u *get_highlight_name(expand_T *xp, int idx);
 char_u *get_highlight_name_ext(expand_T *xp, int idx, int skip_cleared);
+int expand_highlight_group( char_u *pat, expand_T *xp, regmatch_T *rmp, char_u ***matches, int *numMatches);
 void free_highlight_fonts(void);
 void f_hlget(typval_T *argvars, typval_T *rettv);
 void f_hlset(typval_T *argvars, typval_T *rettv);
diff --git a/src/syntax.c b/src/syntax.c
index 34563aa..bbcf02b 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -6372,7 +6372,7 @@
 }
 
 /*
- * Handle command line completion for :match and :echohl command: Add "None"
+ * Handle command line completion for :match and :echohl command: Add "NONE"
  * as highlight group.
  */
     void
diff --git a/src/testdir/test_cmdline.vim b/src/testdir/test_cmdline.vim
index dfebb40..042710c 100644
--- a/src/testdir/test_cmdline.vim
+++ b/src/testdir/test_cmdline.vim
@@ -418,8 +418,8 @@
   hi Aardig ctermfg=green
   call feedkeys(":match \<Tab>\<Home>\"\<CR>", 'xt')
   call assert_equal('"match Aardig', @:)
-  call feedkeys(":match \<S-Tab>\<Home>\"\<CR>", 'xt')
-  call assert_equal('"match none', @:)
+  call feedkeys(":match NON\<Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal('"match NONE', @:)
   call feedkeys(":match | chist\<Tab>\<C-B>\"\<CR>", 'xt')
   call assert_equal('"match | chistory', @:)
 endfunc
@@ -428,20 +428,37 @@
   hi Aardig ctermfg=green
   call feedkeys(":hi \<Tab>\<Home>\"\<CR>", 'xt')
   call assert_equal('"hi Aardig', getreg(':'))
+
+  " hi default
   call feedkeys(":hi default \<Tab>\<Home>\"\<CR>", 'xt')
   call assert_equal('"hi default Aardig', getreg(':'))
-  call feedkeys(":hi clear Aa\<Tab>\<Home>\"\<CR>", 'xt')
-  call assert_equal('"hi clear Aardig', getreg(':'))
-  call feedkeys(":hi li\<S-Tab>\<Home>\"\<CR>", 'xt')
-  call assert_equal('"hi link', getreg(':'))
   call feedkeys(":hi d\<S-Tab>\<Home>\"\<CR>", 'xt')
   call assert_equal('"hi default', getreg(':'))
+  call feedkeys(":hi default link Aa\<Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal('"hi default link Aardig', getreg(':'))
+
+  " hi clear only accepts one parameter
   call feedkeys(":hi c\<S-Tab>\<Home>\"\<CR>", 'xt')
   call assert_equal('"hi clear', getreg(':'))
+  call feedkeys(":hi clear Aa\<Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal('"hi clear Aardig', getreg(':'))
   call feedkeys(":hi clear Aardig Aard\<Tab>\<C-B>\"\<CR>", 'xt')
-  call assert_equal('"hi clear Aardig Aardig', getreg(':'))
-  call feedkeys(":hi Aardig \<Tab>\<C-B>\"\<CR>", 'xt')
-  call assert_equal("\"hi Aardig \t", getreg(':'))
+  call assert_equal("\"hi clear Aardig Aard\<Tab>", getreg(':'))
+  " hi link accepts up to two parameters
+  call feedkeys(":hi li\<S-Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal('"hi link', getreg(':'))
+  call assert_equal('"hi link', getreg(':'))
+  call feedkeys(":hi link Aa\<Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal('"hi link Aardig', getreg(':'))
+  call feedkeys(":hi link Aardig Aard\<Tab>\<C-B>\"\<CR>", 'xt')
+  call assert_equal("\"hi link Aardig Aardig", getreg(':'))
+  call feedkeys(":hi link Aardig Aardig Aard\<Tab>\<C-B>\"\<CR>", 'xt')
+  call assert_equal("\"hi link Aardig Aardig Aard\<Tab>", getreg(':'))
+  " hi link will complete to "NONE" for second parameter
+  call feedkeys(":hi link NON\<Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal("\"hi link NonText", getreg(':'))
+  call feedkeys(":hi link Aardig NON\<Tab>\<Home>\"\<CR>", 'xt')
+  call assert_equal("\"hi link Aardig NONE", getreg(':'))
 
   " A cleared group does not show up in completions.
   hi Anders ctermfg=green
@@ -460,6 +477,102 @@
   call test_override('ALL', 0)
 endfunc
 
+func Test_highlight_group_completion()
+  " Test completing keys
+  call assert_equal('term=', getcompletion('hi Foo ', 'cmdline')[0])
+  call assert_equal('ctermfg=', getcompletion('hi Foo c*fg', 'cmdline')[0])
+  call assert_equal('NONE', getcompletion('hi Foo NON', 'cmdline')[0])
+  set wildoptions+=fuzzy
+  call assert_equal('ctermbg=', getcompletion('hi Foo cmbg', 'cmdline')[0])
+  set wildoptions-=fuzzy
+
+  " Test completing the current value
+  hi FooBar term=bold,underline cterm=undercurl ctermfg=lightgray ctermbg=12 ctermul=34
+  call assert_equal('bold,underline', getcompletion('hi FooBar term=', 'cmdline')[0])
+  call assert_equal('undercurl', getcompletion('hi FooBar cterm=', 'cmdline')[0])
+  call assert_equal('7', getcompletion('hi FooBar ctermfg=', 'cmdline')[0])
+  call assert_equal('12', getcompletion('hi FooBar ctermbg=', 'cmdline')[0])
+  call assert_equal('34', getcompletion('hi FooBar ctermul=', 'cmdline')[0])
+
+  " "bold,underline" is unique and creates an extra item. "undercurl" and
+  " should be de-duplicated
+  call assert_equal(len(getcompletion('hi FooBar term=', 'cmdline')),
+        \ 1 + len(getcompletion('hi FooBar cterm=', 'cmdline')))
+
+  " don't complete original value if we have user input already, similar to
+  " behavior in :set <option>=<pattern>
+  call assert_equal(['bold'], getcompletion('hi FooBar term=bol', 'cmdline'))
+  call assert_equal([], getcompletion('hi FooBar ctermfg=1', 'cmdline'))
+
+  " start/stop do not fill their current value now as they are more
+  " complicated
+  hi FooBar start=123 stop=234
+  call assert_equal([], getcompletion('hi FooBar start=', 'cmdline'))
+  call assert_equal([], getcompletion('hi FooBar stop=', 'cmdline'))
+
+  if has("gui") || has("termguicolors")
+    hi FooBar gui=italic guifg=#112233 guibg=brown1 guisp=green
+    call assert_equal('italic', getcompletion('hi FooBar gui=', 'cmdline')[0])
+    call assert_equal('#112233', getcompletion('hi FooBar guifg=', 'cmdline')[0])
+    call assert_equal('brown1', getcompletion('hi FooBar guibg=', 'cmdline')[0])
+    call assert_equal('green', getcompletion('hi FooBar guisp=', 'cmdline')[0])
+
+    " Check that existing value is de-duplicated and doesn't show up later
+    call assert_equal(1, count(getcompletion('hi FooBar guibg=', 'cmdline'), 'brown1'))
+  endif
+
+  " Test completing attributes
+  call assert_equal(['underdouble', 'underdotted'], getcompletion('hi DoesNotExist term=un*erdo*', 'cmdline'))
+  call assert_equal('NONE', getcompletion('hi DoesNotExist cterm=NON', 'cmdline')[0])
+  call assert_equal('NONE', getcompletion('hi DoesNotExist cterm=', 'cmdline')[-1]) " NONE should be at the end and not sorted
+  call assert_equal('bold', getcompletion('hi DoesNotExist cterm=underline,bo', 'cmdline')[0]) " complete after comma
+  if has("gui") || has("termguicolors")
+    set wildoptions+=fuzzy
+    call assert_equal('italic', getcompletion('hi DoesNotExist gui=itic', 'cmdline')[0])
+    set wildoptions-=fuzzy
+  endif
+
+  " Test completing cterm colors
+  call assert_equal('fg', getcompletion('hi FooBar ctermbg=f*g', 'cmdline')[0])
+  call assert_equal('fg', getcompletion('hi DoesNotExist ctermbg=f*g', 'cmdline')[0])
+  call assert_equal('NONE', getcompletion('hi FooBar ctermfg=NON', 'cmdline')[0])
+  call assert_equal('NONE', getcompletion('hi DoesNotExist ctermfg=NON', 'cmdline')[0])
+  set wildoptions+=fuzzy
+  call assert_equal('Black', getcompletion('hi FooBar ctermul=blck', 'cmdline')[0])
+  call assert_equal('Black', getcompletion('hi DoesNotExist ctermul=blck', 'cmdline')[0])
+  set wildoptions-=fuzzy
+
+  " Test completing gui colors
+  if has("gui") || has("termguicolors")
+    call assert_equal('fg', getcompletion('hi FooBar guibg=f*g', 'cmdline')[0])
+    call assert_equal('fg', getcompletion('hi DoesNotExist guibg=f*g', 'cmdline')[0])
+    call assert_equal('NONE', getcompletion('hi FooBar guifg=NON', 'cmdline')[0])
+    call assert_equal('NONE', getcompletion('hi DoesNotExist guifg=NON', 'cmdline')[0])
+    set wildoptions=fuzzy
+    call assert_equal('limegreen', getcompletion('hi FooBar guisp=limgrn', 'cmdline')[0])
+    call assert_equal('limegreen', getcompletion('hi DoesNotExist guisp=limgrn', 'cmdline')[0])
+    set wildoptions-=fuzzy
+
+    " Test pruning bad color names with space. Vim doesn't support them.
+    let v:colornames['foobar with space'] = '#123456'
+    let v:colornames['foobarwithoutspace'] = '#234567'
+    call assert_equal(['foobarwithoutspace'], getcompletion('hi FooBar guibg=foobarw', 'cmdline'))
+
+    " Test specialized sorting. First few items are special values that
+    " go first, after that it's a sorted list of color names.
+    call assert_equal(['fg','bg','NONE'], getcompletion('hi DoesNotExist guifg=', 'cmdline')[0:2])
+    let completed_colors=getcompletion('hi DoesNotExist guifg=', 'cmdline')[3:]
+    let gui_colors_no_space=filter(copy(v:colornames), {key,val -> match(key, ' ')==-1})
+    call assert_equal(len(gui_colors_no_space), len(completed_colors))
+    call assert_equal(sort(copy(completed_colors)), completed_colors)
+
+    " Test that the specialized sorting still works if we have some pattern matches
+    let completed_colors=getcompletion('hi DoesNotExist guifg=*blue*', 'cmdline')
+    call assert_equal(sort(copy(completed_colors)), completed_colors)
+    call assert_equal('aliceblue', completed_colors[0])
+  endif
+endfunc
+
 func Test_getcompletion()
   let groupcount = len(getcompletion('', 'event'))
   call assert_true(groupcount > 0)
diff --git a/src/testdir/test_syntax.vim b/src/testdir/test_syntax.vim
index 748c2bd..49f8833 100644
--- a/src/testdir/test_syntax.vim
+++ b/src/testdir/test_syntax.vim
@@ -209,7 +209,7 @@
 
 func Test_echohl_completion()
   call feedkeys(":echohl no\<C-A>\<C-B>\"\<CR>", 'tx')
-  call assert_equal('"echohl NonText Normal none', @:)
+  call assert_equal('"echohl NONE NonText Normal', @:)
 endfunc
 
 func Test_syntax_arg_skipped()
diff --git a/src/version.c b/src/version.c
index 948abdf..87ed059 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1138,
+/**/
     1137,
 /**/
     1136,
diff --git a/src/vim.h b/src/vim.h
index e0c40c9..2ebc8fe 100644
--- a/src/vim.h
+++ b/src/vim.h
@@ -846,6 +846,7 @@
 #define EXPAND_DIRS_IN_CDPATH	59
 #define EXPAND_SHELLCMDLINE	60
 #define EXPAND_FINDFUNC		61
+#define EXPAND_HIGHLIGHT_GROUP  62
 
 
 // Values for exmode_active (0 is no exmode)