patch 9.1.1355: The pum_redraw() function is too complex
Commit:
https://github.com/vim/vim/commit/c3e71d4da6633f2804ef3212dcd6ac615580417c
Author: glepnir <glephun...@gmail.com>
Date: Tue Apr 29 18:27:05 2025 +0200
patch 9.1.1355: The pum_redraw() function is too complex
Problem: The pum_redraw function is too complex and difficult to
maintain with nested loops and mixed responsibilities handling
both RTL and LTR text rendering.
Solution: Extracted core rendering logic into dedicated helper functions
(pum_display_rtl_text, pum_display_ltr_text, pum_draw_scrollbar,
pum_process_item) while preserving the original behavior. This
improves code readability and maintainability (glepnir).
closes: #17204
Signed-off-by: glepnir <glephun...@gmail.com>
Signed-off-by: Christian Brabandt <c...@256bit.org>
diff --git a/src/popupmenu.c b/src/popupmenu.c
index 38ab24bce..15d899c9f 100644
--- a/src/popupmenu.c
+++ b/src/popupmenu.c
@@ -574,6 +574,301 @@ pum_user_attr_combine(int idx, int type, int attr)
return user_attr[type] > 0 ? hl_combine_attr(attr, user_attr[type]) : attr;
}
+#ifdef FEAT_RIGHTLEFT
+/*
+ * Display RTL text with proper attributes in the popup menu.
+ * Returns the adjusted column position after drawing.
+ */
+ static int
+pum_display_rtl_text(
+ int row,
+ int col,
+ char_u *st,
+ int attr,
+ int *attrs,
+ int width,
+ int width_limit,
+ int totwidth,
+ int next_isempty)
+{
+ char_u *rt;
+ int cells;
+ int over_cell = 0;
+ int truncated = FALSE;
+ int pad = next_isempty ? 0 : 2;
+ int remaining;
+ int truncrl = curwin->w_fill_chars.truncrl != NUL
+ ? curwin->w_fill_chars.truncrl : '<';
+
+ if (st == NULL)
+ return col;
+
+ rt = reverse_text(st);
+ if (rt == NULL)
+ {
+ VIM_CLEAR(st);
+ return col;
+ }
+
+ char_u *rt_start = rt;
+ cells = mb_string2cells(rt, -1);
+ truncated = width_limit == p_pmw && width_limit - totwidth < cells + pad;
+
+ // only draw the text that fits
+ if (cells > width_limit)
+ {
+ do
+ {
+ cells -= has_mbyte ? (*mb_ptr2cells)(rt) : 1;
+ MB_PTR_ADV(rt);
+ } while (cells > width_limit);
+
+ if (cells < width_limit)
+ {
+ // Most left character requires 2-cells
+ // but only 1 cell is available on screen.
+ // Put a '<' on the left of the pum item.
+ *(--rt) = '<';
+ cells++;
+ }
+ }
+
+ if (truncated)
+ {
+ char_u *orig_rt = rt;
+ int size = 0;
+
+ remaining = width_limit - totwidth - 1;
+ cells = mb_string2cells(rt, -1);
+ if (cells > remaining)
+ {
+ while (cells > remaining)
+ {
+ MB_PTR_ADV(orig_rt);
+ cells -= has_mbyte ? (*mb_ptr2cells)(orig_rt) : 1;
+ }
+ }
+ size = (int)STRLEN(orig_rt);
+ if (cells < remaining)
+ over_cell = remaining - cells;
+
+ cells = mb_string2cells(orig_rt, size);
+ width = cells + over_cell + 1;
+ rt = orig_rt;
+
+ screen_putchar(truncrl, row, col - width + 1, attr);
+
+ if (over_cell > 0)
+ screen_fill(row, row + 1, col - width + 2,
+ col - width + 2 + over_cell, ' ', ' ', attr);
+ }
+
+ if (attrs == NULL)
+ screen_puts_len(rt, (int)STRLEN(rt), row, col - cells + 1, attr);
+ else
+ pum_screen_puts_with_attrs(row, col - cells + 1, cells, rt,
+ (int)STRLEN(rt), attrs);
+
+ vim_free(rt_start);
+ VIM_CLEAR(st);
+ return col - width;
+}
+#endif
+
+/*
+ * Display LTR text with proper attributes in the popup menu.
+ * Returns the adjusted column position after drawing.
+ */
+ static int
+pum_display_ltr_text(
+ int row,
+ int col,
+ char_u *st,
+ int attr,
+ int *attrs,
+ int width, // width already calculated in outer loop
+ int width_limit,
+ int totwidth,
+ int next_isempty)
+{
+ int size;
+ int cells;
+ char_u *st_end = NULL;
+ int over_cell = 0;
+ int pad = next_isempty ? 0 : 2;
+ int truncated;
+ int remaining;
+ int trunc = curwin->w_fill_chars.trunc != NUL
+ ? curwin->w_fill_chars.trunc : '>';
+
+ if (st == NULL)
+ return col;
+
+ size = (int)STRLEN(st);
+ cells = (*mb_string2cells)(st, size);
+ truncated = width_limit == p_pmw && width_limit - totwidth < cells + pad;
+
+ // only draw the text that fits
+ while (size > 0 && col + cells > width_limit + pum_col)
+ {
+ --size;
+ if (has_mbyte)
+ {
+ size -= (*mb_head_off)(st, st + size);
+ cells -= (*mb_ptr2cells)(st + size);
+ }
+ else
+ --cells;
+ }
+
+ // truncated
+ if (truncated)
+ {
+ remaining = width_limit - totwidth - 1;
+ if (cells > remaining)
+ {
+ st_end = st + size;
+ while (st_end > st && cells > remaining)
+ {
+ MB_PTR_BACK(st, st_end);
+ cells -= has_mbyte ? (*mb_ptr2cells)(st_end) : 1;
+ }
+ size = st_end - st;
+ }
+
+ if (cells < remaining)
+ over_cell = remaining - cells;
+ cells = mb_string2cells(st, size);
+ width = cells + over_cell + 1;
+ }
+
+ if (attrs == NULL)
+ screen_puts_len(st, size, row, col, attr);
+ else
+ pum_screen_puts_with_attrs(row, col, cells, st, size, attrs);
+
+ if (truncated)
+ {
+ if (over_cell > 0)
+ screen_fill(row, row + 1, col + cells,
+ col + cells + over_cell, ' ', ' ', attr);
+
+ screen_putchar(trunc, row, col + cells + over_cell, attr);
+ }
+
+ VIM_CLEAR(st);
+ return col + width;
+}
+
+
+/*
+ * Process and display a single popup menu item (text/kind/extra).
+ * Returns the new column position after drawing.
+ */
+ static int
+pum_process_item(
+ int row,
+ int col,
+ int idx,
+ int j, // Current position in order array
+ int *order, // Order array
+ hlf_T hlf,
+ int attr,
+ int *totwidth_ptr,
+ int next_isempty)
+{
+ int item_type = order[j];
+ char_u *s = NULL;
+ char_u *p = pum_get_item(idx, item_type);
+ int width = 0; // item width
+ int w; // char width
+
+ for ( ; ; MB_PTR_ADV(p))
+ {
+ if (s == NULL)
+ s = p;
+ w = ptr2cells(p);
+ if (*p != NUL && *p != TAB && *totwidth_ptr + w <= pum_width)
+ {
+ width += w;
+ continue;
+ }
+
+ // Display the text that fits or comes before a Tab.
+ // First convert it to printable characters.
+ char_u *st;
+ int *attrs = NULL;
+ int saved = *p;
+
+ if (saved != NUL)
+ *p = NUL;
+ st = transstr(s);
+ if (saved != NUL)
+ *p = saved;
+
+ if (item_type == CPT_ABBR)
+ attrs = pum_compute_text_attrs(st, hlf,
+ pum_array[idx].pum_user_abbr_hlattr);
+#ifdef FEAT_RIGHTLEFT
+ if (pum_rl)
+ col = pum_display_rtl_text(row, col, st, attr, attrs,
+ width, pum_width, *totwidth_ptr, next_isempty);
+ else
+#endif
+ col = pum_display_ltr_text(row, col, st, attr, attrs,
+ width, pum_width, *totwidth_ptr, next_isempty);
+
+ if (attrs != NULL)
+ VIM_CLEAR(attrs);
+
+ if (*p != TAB)
+ break;
+
+ // Display two spaces for a Tab.
+#ifdef FEAT_RIGHTLEFT
+ if (pum_rl)
+ {
+ screen_puts_len((char_u *)" ", 2, row, col - 1, attr);
+ col -= 2;
+ }
+ else
+#endif
+ {
+ screen_puts_len((char_u *)" ", 2, row, col, attr);
+ col += 2;
+ }
+ *totwidth_ptr += 2;
+ s = NULL; // start text at next char
+ width = 0;
+ }
+
+ return col;
+}
+
+/*
+ * Draw the scrollbar for the popup menu.
+ */
+ static void
+pum_draw_scrollbar(
+ int row,
+ int i,
+ int thumb_pos,
+ int thumb_height)
+{
+ if (pum_scrollbar <= 0)
+ return;
+
+ int attr = (i >= thumb_pos && i < thumb_pos + thumb_height) ?
+ highlight_attr[HLF_PST] : highlight_attr[HLF_PSB];
+
+#ifdef FEAT_RIGHTLEFT
+ if (pum_rl)
+ screen_putchar(' ', row, pum_col - pum_width, attr);
+ else
+#endif
+ screen_putchar(' ', row, pum_col + pum_width, attr);
+}
+
/*
* Redraw the popup menu, using "pum_first" and "pum_selected".
*/
@@ -582,16 +877,13 @@ pum_redraw(void)
{
int row = pum_row;
int col;
- int attr_scroll = highlight_attr[HLF_PSB];
- int attr_thumb = highlight_attr[HLF_PST];
hlf_T *hlfs; // array used for highlights
hlf_T hlf;
int attr;
int i, j;
int idx;
- char_u *s;
char_u *p = NULL;
- int totwidth, width, w; // total-width item-width
char-width
+ int totwidth;
int thumb_pos = 0;
int thumb_height = 1;
int item_type;
@@ -604,15 +896,6 @@ pum_redraw(void)
int last_isabbr = FALSE;
int orig_attr = -1;
int scroll_range = pum_size - pum_height;
- int remaining = 0;
- int fcs_trunc;
-
-#ifdef FEAT_RIGHTLEFT
- if (pum_rl)
- fcs_trunc = curwin->w_fill_chars.truncrl;
- else
-#endif
- fcs_trunc = curwin->w_fill_chars.trunc;
hlf_T hlfsNorm[3];
hlf_T hlfsSel[3];
@@ -688,221 +971,15 @@ pum_redraw(void)
orig_attr = attr;
if (item_type < 2) // try combine attr with user custom
attr = pum_user_attr_combine(idx, item_type, attr);
- width = 0;
- s = NULL;
p = pum_get_item(idx, item_type);
if (j + 1 < 3)
next_isempty = pum_get_item(idx, order[j + 1]) == NULL;
if (p != NULL)
- for ( ; ; MB_PTR_ADV(p))
- {
- if (s == NULL)
- s = p;
- w = ptr2cells(p);
- if (*p != NUL && *p != TAB && totwidth + w <= pum_width)
- {
- width += w;
- continue;
- }
-
- // Display the text that fits or comes before a Tab.
- // First convert it to printable characters.
- char_u *st;
- int *attrs = NULL;
- int saved = *p;
-
- if (saved != NUL)
- *p = NUL;
- st = transstr(s);
- if (saved != NUL)
- *p = saved;
-
- if (item_type == CPT_ABBR)
- attrs = pum_compute_text_attrs(st, hlf,
- pum_array[idx].pum_user_abbr_hlattr);
-#ifdef FEAT_RIGHTLEFT
- if (pum_rl)
- {
- if (st != NULL)
- {
- char_u *rt = reverse_text(st);
-
- if (rt != NULL)
- {
- char_u *rt_start = rt;
- int cells;
- int over_cell = 0;
- int truncated = FALSE;
- int pad = next_isempty ? 0 : 2;
-
- cells = mb_string2cells(rt , -1);
- truncated = pum_width == p_pmw
- && pum_width - totwidth < cells +
pad;
-
- // only draw the text that fits
- if (cells > pum_width)
- {
- do
- {
- cells -= has_mbyte
- ? (*mb_ptr2cells)(rt) : 1;
- MB_PTR_ADV(rt);
- } while (cells > pum_width);
-
- if (cells < pum_width)
- {
- // Most left character requires 2-cells
- // but only 1 cell is available on
- // screen. Put a '<' on the left of
- // the pum item.
- *(--rt) = '<';
- cells++;
- }
- }
-
- if (truncated)
- {
- char_u *orig_rt = rt;
- int size = 0;
-
- remaining = pum_width - totwidth - 1;
- cells = mb_string2cells(rt, -1);
- if (cells > remaining)
- {
- while (cells > remaining)
- {
- MB_PTR_ADV(orig_rt);
- cells -= has_mbyte ?
(*mb_ptr2cells)(orig_rt) : 1;
- }
- }
- size = (int)STRLEN(orig_rt);
- if (cells < remaining)
- over_cell = remaining - cells;
-
- cells = mb_string2cells(orig_rt, size);
- width = cells + over_cell + 1;
- rt = orig_rt;
-
- if (fcs_trunc != NUL)
- screen_putchar(fcs_trunc, row, col -
width + 1, attr);
- else
- screen_putchar('<', row, col - width +
1, attr);
-
- if (over_cell > 0)
- screen_fill(row, row + 1, col - width +
2,
- col - width + 2 +
over_cell, ' ', ' ', attr);
- }
-
- if (attrs == NULL)
- screen_puts_len(rt, (int)STRLEN(rt), row,
- col - cells + 1, attr);
- else
- pum_screen_puts_with_attrs(row,
- col - cells + 1, cells, rt,
- (int)STRLEN(rt), attrs);
-
- vim_free(rt_start);
- }
- vim_free(st);
- }
- col -= width;
- }
- else
-#endif
- {
- if (st != NULL)
- {
- int size = (int)STRLEN(st);
- int cells = (*mb_string2cells)(st, size);
- char_u *st_end = NULL;
- int over_cell = 0;
- int pad = next_isempty ? 0 : 2;
- int truncated = pum_width == p_pmw
- && pum_width - totwidth < cells +
pad;
-
- // only draw the text that fits
- while (size > 0
- && col + cells > pum_width + pum_col)
- {
- --size;
- if (has_mbyte)
- {
- size -= (*mb_head_off)(st, st + size);
- cells -= (*mb_ptr2cells)(st + size);
- }
- else
- --cells;
- }
-
- // truncated
- if (truncated)
- {
- remaining = pum_width - totwidth - 1;
- if (cells > remaining)
- {
- st_end = st + size;
- while (st_end > st && cells > remaining)
- {
- MB_PTR_BACK(st, st_end);
- cells -= has_mbyte ?
(*mb_ptr2cells)(st_end) : 1;
- }
- size = st_end - st;
- }
-
- if (cells < remaining)
- over_cell = remaining - cells;
- cells = mb_string2cells(st, size);
- width = cells + over_cell + 1;
- }
-
- if (attrs == NULL)
- screen_puts_len(st, size, row, col, attr);
- else
- pum_screen_puts_with_attrs(row, col, cells,
- st, size, attrs);
- if (truncated)
- {
- if (over_cell > 0)
- screen_fill(row, row + 1, col + cells,
- col + cells + over_cell, ' ', ' ',
attr);
- if (fcs_trunc != NUL)
- screen_putchar(fcs_trunc, row,
- col + cells + over_cell, attr);
- else
- screen_putchar('>', row,
- col + cells + over_cell, attr);
- }
-
- vim_free(st);
- }
- col += width;
- }
-
- if (attrs != NULL)
- VIM_CLEAR(attrs);
-
- if (*p != TAB)
- break;
-
- // Display two spaces for a Tab.
-#ifdef FEAT_RIGHTLEFT
- if (pum_rl)
- {
- screen_puts_len((char_u *)" ", 2, row, col - 1, attr);
- col -= 2;
- }
- else
-#endif
- {
- screen_puts_len((char_u *)" ", 2, row, col, attr);
- col += 2;
- }
- totwidth += 2;
- s = NULL; // start text at next char
- width = 0;
- }
+ // Process and display the item
+ col = pum_process_item(row, col, idx, j, order, hlf, attr,
+ &totwidth, next_isempty);
if (j > 0)
n = items_width_array[order[1]] + (last_isabbr ? 0 : 1);
@@ -940,19 +1017,7 @@ pum_redraw(void)
#endif
screen_fill(row, row + 1, col, pum_col + pum_width, ' ', ' ',
orig_attr);
- if (pum_scrollbar > 0)
- {
-#ifdef FEAT_RIGHTLEFT
- if (pum_rl)
- screen_putchar(' ', row, pum_col - pum_width,
- i >= thumb_pos && i < thumb_pos + thumb_height
- ? attr_thumb : attr_scroll);
- else
-#endif
- screen_putchar(' ', row, pum_col + pum_width,
- i >= thumb_pos && i < thumb_pos + thumb_height
- ? attr_thumb : attr_scroll);
- }
+ pum_draw_scrollbar(row, i, thumb_pos, thumb_height);
++row;
}
diff --git a/src/version.c b/src/version.c
index f93213b04..0c731a88d 100644
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 1355,
/**/
1354,
/**/
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to vim_dev+unsubscr...@googlegroups.com.
To view this discussion visit
https://groups.google.com/d/msgid/vim_dev/E1u9o4t-00FPOd-Pm%40256bit.org.
