Ensure a minimum width for the completion popup menu

[languitar]

Following the discussions in neovim/neovim#5649, I open this issue here as the upstream project. This is my original text from the neovim issue:

In case the completion is triggered close to the right border of the terminal, it might look something like this:

In some situations this makes the entries indistinguishable and completion pretty useless.

It would be nice if there was an option to specify a minimum width of the completion popup and in case the width cannot be held, the origin of the popup could be shifted to the left, away from the cursor position. As there is always only one of these popups, I don't see a strong reason why it always has to appear at the cursor location.

This has also been requested at the Debian bug tracker: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=450481 and @jamessan has indicated that an unfinished patch exists in the neovim issue.

[brammool]

Johannes Wienke wrote:

Following the discussions in neovim/neovim#5649, I open this issue here as the upstream project. This is my original text from the neovim issue: In case the completion is triggered close to the right border of the terminal, it might look something like this:  In some situations this makes the entries indistinguishable and completion pretty useless. It would be nice if there was an option to specify a minimum width of the completion popup and in case the width cannot be held, the origin of the popup could be shifted to the left, away from the cursor position. As there is always only one of these popups, I don't see a strong reason why it always has to appear at the cursor location. This has also been requested at the Debian bug tracker: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=450481 and @jamessan has indicated that an unfinished patch exists in the neovim issue.

We already have the 'pumheight' option, we could add a 'pumwidth' option. Currently it's hard coded as PUM_DEF_WIDTH. Should not be too difficult.

…

-- ARTHUR: What are you going to do. bleed on me? "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[chrisbra]

You mean like this?

From 6e624279cda8f8dcad17253a54b03f06dca19dd8 Mon Sep 17 00:00:00 2001
From: Christian Brabandt <cb@256bit.org>
Date: Mon, 13 Nov 2017 17:08:46 +0100
Subject: [PATCH] add 'pumwidth' option

fixes #2314
allows to define the max width for the popup menu.

Most of this has been done by @jamessan
---
 runtime/doc/options.txt | 10 +++++++++
 src/option.c            |  7 +++++++
 src/option.h            |  1 +
 src/popupmnu.c          | 55 ++++++++++++++++++++++++++++++++++++++++++-------
 src/proto/popupmnu.pro  |  1 +
 5 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index 1af19606d..61397f3ca 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -6010,6 +6010,16 @@ A jump table for the options with a short description can be found at |Q_op|.
 			{not in Vi}
 	Determines the maximum number of items to show in the popup menu for
 	Insert mode completion.  When zero as much space as available is used.
+	|ins-completion-menu|.
+
+						*'pumwidth'* *'pw'*
+'pumwidth' 'pw'	        number	(default 0)
+			global
+			{not available when compiled without the
+			|+insert_expand| feature}
+			{not in Vi}
+	Determines the maximum width to show in the popup menu for
+	Insert mode completion.  When zero as much space as available is used.
 	|ins-completion-menu|.
 
 						*'pythondll'*
diff --git a/src/option.c b/src/option.c
index 4f25c1ff4..b84036fba 100644
--- a/src/option.c
+++ b/src/option.c
@@ -2228,6 +2228,13 @@ static struct vimoption options[] =
 			    (char_u *)&p_ph, PV_NONE,
 #else
 			    (char_u *)NULL, PV_NONE,
+#endif
+			    {(char_u *)0L, (char_u *)0L} SCRIPTID_INIT},
+    {"pumwidth",    "pw",   P_NUM|P_VI_DEF,
+#ifdef FEAT_INS_EXPAND
+			    (char_u *)&p_pw, PV_NONE,
+#else
+			    (char_u *)NULL, PV_NONE,
 #endif
 			    {(char_u *)0L, (char_u *)0L} SCRIPTID_INIT},
     {"pythonthreedll",  NULL,   P_STRING|P_EXPAND|P_VI_DEF|P_SECURE,
diff --git a/src/option.h b/src/option.h
index f9972f21a..30712b494 100644
--- a/src/option.h
+++ b/src/option.h
@@ -419,6 +419,7 @@ EXTERN int	p_cp;		/* 'compatible' */
 #ifdef FEAT_INS_EXPAND
 EXTERN char_u	*p_cot;		/* 'completeopt' */
 EXTERN long	p_ph;		/* 'pumheight' */
+EXTERN long	p_pw;		/* 'pumwidth' */
 #endif
 EXTERN char_u	*p_cpo;		/* 'cpoptions' */
 #ifdef FEAT_CSCOPE
diff --git a/src/popupmnu.c b/src/popupmnu.c
index ec75281e7..f74a51b7d 100644
--- a/src/popupmnu.c
+++ b/src/popupmnu.c
@@ -66,7 +66,7 @@ pum_display(
 
     do
     {
-	def_width = PUM_DEF_WIDTH;
+	def_width = pum_get_height();
 	max_width = 0;
 	kind_width = 0;
 	extra_width = 0;
@@ -209,10 +209,10 @@ pum_display(
 	if (def_width < max_width)
 	    def_width = max_width;
 
-	if (((col < Columns - PUM_DEF_WIDTH || col < Columns - max_width)
+	if (((col < Columns - pum_get_width() || col < Columns - max_width)
 #ifdef FEAT_RIGHTLEFT
 		    && !curwin->w_p_rl)
-		|| (curwin->w_p_rl && (col > PUM_DEF_WIDTH || col > max_width)
+		|| (curwin->w_p_rl && (col > pum_get_width() || col > max_width)
 #endif
 	   ))
 	{
@@ -227,12 +227,44 @@ pum_display(
 		pum_width = Columns - pum_col - pum_scrollbar;
 
 	    if (pum_width > max_width + kind_width + extra_width + 1
-						     && pum_width > PUM_DEF_WIDTH)
+						     && pum_width > pum_get_width())
 	    {
 		pum_width = max_width + kind_width + extra_width + 1;
-		if (pum_width < PUM_DEF_WIDTH)
-		    pum_width = PUM_DEF_WIDTH;
+		if (pum_width < pum_get_width())
+		    pum_width = pum_get_width();
 	    }
+	    else if (((col > pum_get_width() || col > max_width)
+#ifdef FEAT_RIGHTLEFT
+		&& !curwin->w_p_rl)
+		|| (curwin->w_p_rl && (col < Columns - pum_get_width()
+			|| col < Columns - max_width)
+#endif
+	    ))
+	    {
+	    /* align right pum edge with "col" */
+#ifdef FEAT_RIGHTLEFT
+		if (curwin->w_p_rl)
+		    pum_col = col + max_width + pum_scrollbar + 1;
+		else
+#endif
+		    pum_col = col - max_width - pum_scrollbar;
+
+#ifdef FEAT_RIGHTLEFT
+		if (curwin->w_p_rl)
+		    pum_width = W_ENDCOL(curwin) - pum_col - pum_scrollbar + 1;
+		else
+#endif
+		    pum_width = pum_col - pum_scrollbar;
+
+		if (pum_width > max_width + kind_width + extra_width + 1
+			    && pum_width > pum_get_width())
+		{
+		    pum_width = max_width + kind_width + extra_width + 1;
+		    if (pum_width < pum_get_width())
+			pum_width = pum_get_width();
+		}
+	    }
+
 	}
 	else if (Columns < def_width)
 	{
@@ -247,8 +279,8 @@ pum_display(
 	}
 	else
 	{
-	    if (max_width > PUM_DEF_WIDTH)
-		max_width = PUM_DEF_WIDTH;	/* truncate */
+	    if (max_width > pum_get_width())
+		max_width = pum_get_width();	/* truncate */
 #ifdef FEAT_RIGHTLEFT
 	    if (curwin->w_p_rl)
 		pum_col = max_width - 1;
@@ -756,4 +788,11 @@ pum_get_height(void)
     return pum_height;
 }
 
+/* Return the minimum width of the popup menu */
+    int
+pum_get_width(void)
+{
+    return p_pw ? p_pw : PUM_DEF_WIDTH;
+}
+
 #endif
diff --git a/src/proto/popupmnu.pro b/src/proto/popupmnu.pro
index 423076c3e..c0531f48d 100644
--- a/src/proto/popupmnu.pro
+++ b/src/proto/popupmnu.pro
@@ -5,4 +5,5 @@ void pum_undisplay(void);
 void pum_clear(void);
 int pum_visible(void);
 int pum_get_height(void);
+int pum_get_width(void);
 /* vim: set ft=c : */
-- 
2.14.1

[vim-ml]

Hi, Op 13 nov. 2017 17:32 schreef "Christian Brabandt" < vim-dev-github@256bit.org>: *'pythondll'* diff --git a/src/popupmnu.c b/src/popupmnu.c index ec75281e7..f74a51b7d 100644--- a/src/popupmnu.c+++ b/src/popupmnu.c@@ -66,7 +66,7 @@ pum_display( do {- def_width = PUM_DEF_WIDTH;+ def_width = pum_get_height(); max_width = 0; kind_width = 0; extra_width = 0;@@ -209,10 +209,10 @@ pum_display( if (def_width < max_width) def_width = max_width; This looks wrong, mixing width and height. Christ van Willegen

[brammool]

Christian wrote:

You mean like this?

Thanks. But this looks wrong: - def_width = PUM_DEF_WIDTH; + def_width = pum_get_height();

…

-- I recommend ordering large cargo containers of paper towels to make up whatever budget underruns you have. Paper products are always useful and they have the advantage of being completely flushable if you need to make room in the storage area later. (Scott Adams - The Dilbert principle) /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[chrisbra]

ah yeah, I messed that up. I'll post a proper patch later

[languitar]

What's the state here? Any chance to get this going again?

[chrisbra]

It is still on my list to do. Unfortunately my fucked up private life come into play so had no chance to fix it. I don't expect this to be any better in the foreseeable future. And then I also have to take care of my profession, which also needs much more time then expected. So sorry, no progress here.

[chrisbra]

updated patch

[brammool]

I am including the patch, but had to fix several problems. The popup test crashed.
Looks like a few more improvements are needed, the code is a bit messy and lacks comments about what the intention of the various if-else blocks are. It should be possible to improve and simplify the logic.

[chrisbra]

On Sa, 10 Feb 2018, Bram Moolenaar wrote: I am including the patch, but had to fix several problems. The popup test crashed. Looks like a few more improvements are needed, the code is a bit messy and lacks comments about what the intention of the various if-else blocks are. It should be possible to improve and simplify the logic.

Thanks for fixing my flaws. sorry for that, will check test results better in the future. Christian -- Das Alter hört sich gern, auch wenn es nichts zu sagen hat. -- Johann Wolfgang von Goethe

[micbou]

I think this patch should be reverted. See issue #2640.

I don't see a strong reason why it always has to appear at the cursor location.

There is a good reason why users expect the completion menu to be at the cursor position: it's where they are looking at.