From c3e71d4da6633f2804ef3212dcd6ac615580417c Mon Sep 17 00:00:00 2001 From: glepnir Date: Tue, 29 Apr 2025 18:27:05 +0200 Subject: [PATCH] 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 Signed-off-by: Christian Brabandt --- src/popupmenu.c | 535 +++++++++++++++++++++++++++--------------------- src/version.c | 2 + 2 files changed, 302 insertions(+), 235 deletions(-) diff --git a/src/popupmenu.c b/src/popupmenu.c index 38ab24bcec..15d899c9f9 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 f93213b041..0c731a88dd 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, /**/