patch 9.1.0921: popupmenu logic is a bit convoluted

Problem:  popupmenu logic is a bit convoluted
Solution: slightly refactor logic and use MIN/MAX() macros to simplify
          (glepnir)

Define the MAX/MIN macros. Since we support some older platforms, C
compilers may not be as smart. This helps reduce unnecessary if
statements and redundant ternary expressions. Pre-calculate some
expressions by defining variables. Remove unnecessary parentheses.
Adjust certain lines to avoid exceeding 80 columns.

closes: #16205

Signed-off-by: glepnir <glephunter@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

src/ex_getln.c

@@ -13,10 +13,6 @@
 
 #include "vim.h"
 
-#ifndef MAX
-# define MAX(x,y) ((x) > (y) ? (x) : (y))
-#endif
-
 // Return value when handling keys in command-line mode.
 #define CMDLINE_NOT_CHANGED	1
 #define CMDLINE_CHANGED		2

src/popupmenu.c

@@ -100,6 +100,9 @@ pum_display(
     int		cursor_col;
     int		above_row;
     int		below_row;
+    int		cline_visible_offset;
+    int		content_width;
+    int		right_edge_col;
     int		redo_count = 0;
 #if defined(FEAT_QUICKFIX)
     win_T	*pvwin;
@@ -150,10 +153,7 @@ pum_display(
 	/*
 	 * Figure out the size and position of the pum.
 	 */
-	if (size < PUM_DEF_HEIGHT)
+	pum_height = MIN(size, PUM_DEF_HEIGHT);
-	    pum_height = size;
-	else
-	    pum_height = PUM_DEF_HEIGHT;
 	if (p_ph > 0 && pum_height > p_ph)
 	    pum_height = p_ph;
 
@@ -168,13 +168,8 @@ pum_display(
 		// for cmdline pum, no need for context lines
 		context_lines = 0;
 	    else
-	    {
 		// Leave two lines of context if possible
-		if (curwin->w_wrow - curwin->w_cline_row >= 2)
+		context_lines = MIN(2, curwin->w_wrow - curwin->w_cline_row);
-		    context_lines = 2;
-		else
-		    context_lines = curwin->w_wrow - curwin->w_cline_row;
-	    }
 
 	    if (pum_win_row >= size + context_lines)
 	    {
@@ -203,19 +198,13 @@ pum_display(
 	    {
 		// Leave two lines of context if possible
 		validate_cheight();
-		if (curwin->w_cline_row
+		cline_visible_offset = curwin->w_cline_row +
-				+ curwin->w_cline_height - curwin->w_wrow >= 3)
+				    curwin->w_cline_height - curwin->w_wrow;
-		    context_lines = 3;
+		context_lines = MIN(3, cline_visible_offset);
-		else
-		    context_lines = curwin->w_cline_row
-				     + curwin->w_cline_height - curwin->w_wrow;
 	    }
 
 	    pum_row = pum_win_row + context_lines;
-	    if (size > below_row - pum_row)
+	    pum_height = MIN(below_row - pum_row, size);
-		pum_height = below_row - pum_row;
-	    else
-		pum_height = size;
 	    if (p_ph > 0 && pum_height > p_ph)
 		pum_height = p_ph;
 	}
@@ -284,15 +273,10 @@ pum_display(
 #endif
 		pum_width = Columns - pum_col - pum_scrollbar;
 
-	    if (pum_width > max_width + pum_kind_width + pum_extra_width + 1
+	    content_width = max_width + pum_kind_width + pum_extra_width + 1;
-						&& pum_width > p_pw)
+	    if (pum_width > content_width && pum_width > p_pw)
-	    {
+		// Reduce width to fit item
-		// the width is more than needed for the items, make it
+		pum_width = MAX(content_width , p_pw);
-		// narrower
-		pum_width = max_width + pum_kind_width + pum_extra_width + 1;
-		if (pum_width < p_pw)
-		    pum_width = p_pw;
-	    }
 	    else if (((cursor_col > p_pw || cursor_col > max_width)
 #ifdef FEAT_RIGHTLEFT
 			&& !pum_rl)
@@ -313,14 +297,10 @@ pum_display(
 		else if (!pum_rl)
 #endif
 		{
-		    if (curwin->w_wincol > Columns - max_width - pum_scrollbar
+		    right_edge_col = Columns - max_width - pum_scrollbar;
-							  && max_width <= p_pw)
+		    if (curwin->w_wincol > right_edge_col && max_width <= p_pw)
-		    {
 			// use full width to end of the screen
-			pum_col = Columns - max_width - pum_scrollbar;
+			pum_col = MAX(0, right_edge_col);
-			if (pum_col < 0)
-			    pum_col = 0;
-		    }
 		}
 
 #ifdef FEAT_RIGHTLEFT
@@ -346,15 +326,8 @@ pum_display(
 			    pum_width = Columns - pum_col - 1;
 		    }
 		}
-		else if (pum_width > max_width + pum_kind_width
+		else if (pum_width > content_width && pum_width > p_pw)
-							  + pum_extra_width + 1
+		    pum_width = MIN(content_width, p_pw);
-			    && pum_width > p_pw)
-		{
-		    pum_width = max_width + pum_kind_width
-							 + pum_extra_width + 1;
-		    if (pum_width < p_pw)
-			pum_width = p_pw;
-		}
 	    }
 
 	}
@@ -998,8 +971,7 @@ pum_set_selected(int n, int repeat UNUSED)
 	    }
 	}
 	// adjust for the number of lines displayed
-	if (pum_first > pum_size - pum_height)
+	pum_first = MIN(pum_first, pum_size - pum_height);
-	    pum_first = pum_size - pum_height;
 
 #if defined(FEAT_QUICKFIX)
 	/*
@@ -1322,8 +1294,7 @@ pum_may_redraw(void)
 
     if (pum_in_same_position())
     {
-	// window position didn't change, redraw in the same position
+	pum_redraw();  // Redraw window in same position
-	pum_redraw();
     }
     else
     {
@@ -1403,8 +1374,7 @@ pum_position_at_mouse(int min_width)
 	    pum_col = mouse_col;
 	else
 	    // Not enough space, left align with window.
-	    pum_col = (pum_base_width > min_width
+	    pum_col = MIN(pum_base_width, min_width) - 1;
-					     ? min_width : pum_base_width) - 1;
 	pum_width = pum_col + 1;
     }
     else
@@ -1416,8 +1386,7 @@ pum_position_at_mouse(int min_width)
 	    pum_col = mouse_col;
 	else
 	    // Not enough space, right align with window.
-	    pum_col = Columns - (pum_base_width > min_width
+	    pum_col = Columns -  MIN(pum_base_width, min_width);
-						 ? min_width : pum_base_width);
 	pum_width = Columns - pum_col;
     }
 

src/version.c

@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    921,
 /**/
     920,
 /**/