changeset 19265:ce8c47ed54e5 v8.2.0191

patch 8.2.0191: cannot put a terminal in a popup window Commit: https://github.com/vim/vim/commit/219c7d063823498be22aae46dd024d77b5fb2a58 Author: Bram Moolenaar <Bram@vim.org> Date: Sat Feb 1 21:57:29 2020 +0100 patch 8.2.0191: cannot put a terminal in a popup window Problem: Cannot put a terminal in a popup window. Solution: Allow opening a terminal in a popup window. It will always have keyboard focus until closed.
author Bram Moolenaar <Bram@vim.org>
date Sat, 01 Feb 2020 22:00:04 +0100
parents 489e4c8101ba
children 4bb646771aef
files src/drawline.c src/highlight.c src/macros.h src/mouse.c src/optionstr.c src/popupwin.c src/proto/popupwin.pro src/proto/terminal.pro src/terminal.c src/testdir/dumps/Test_terminal_popup_1.dump src/testdir/dumps/Test_terminal_popup_2.dump src/testdir/dumps/Test_terminal_popup_3.dump src/testdir/test_terminal.vim src/version.c src/window.c
diffstat 15 files changed, 310 insertions(+), 64 deletions(-) [+]
line wrap: on
line diff
--- a/src/drawline.c
+++ b/src/drawline.c
@@ -491,7 +491,7 @@ win_line(
 	{
 	    extra_check = TRUE;
 	    get_term_attr = TRUE;
-	    win_attr = term_get_attr(wp->w_buffer, lnum, -1);
+	    win_attr = term_get_attr(wp, lnum, -1);
 	}
 #endif
 
@@ -1419,7 +1419,7 @@ win_line(
 		syntax_attr = 0;
 # ifdef FEAT_TERMINAL
 		if (get_term_attr)
-		    syntax_attr = term_get_attr(wp->w_buffer, lnum, vcol);
+		    syntax_attr = term_get_attr(wp, lnum, vcol);
 # endif
 		// Get syntax attribute.
 		if (has_syntax)
--- a/src/highlight.c
+++ b/src/highlight.c
@@ -3134,8 +3134,9 @@ syn_id2colors(int hl_id, guicolor_T *fgp
 #endif
 
 #if (defined(MSWIN) \
-	&& (!defined(FEAT_GUI_MSWIN) || defined(VIMDLL)) \
-	&& defined(FEAT_TERMGUICOLORS)) || defined(PROTO)
+	    && (!defined(FEAT_GUI_MSWIN) || defined(VIMDLL)) \
+	    && defined(FEAT_TERMGUICOLORS)) \
+	|| defined(FEAT_TERMINAL) || defined(PROTO)
     void
 syn_id2cterm_bg(int hl_id, int *fgp, int *bgp)
 {
--- a/src/macros.h
+++ b/src/macros.h
@@ -345,8 +345,10 @@
 // Give an error in curwin is a popup window and evaluate to TRUE.
 #ifdef FEAT_PROP_POPUP
 # define ERROR_IF_POPUP_WINDOW error_if_popup_window()
+# define ERROR_IF_TERM_POPUP_WINDOW error_if_term_popup_window()
 #else
 # define ERROR_IF_POPUP_WINDOW 0
+# define ERROR_IF_TERM_POPUP_WINDOW 0
 #endif
 
 
--- a/src/mouse.c
+++ b/src/mouse.c
@@ -1736,6 +1736,11 @@ retnomove:
 # endif
 	}
 #endif
+#if defined(FEAT_PROP_POPUP) && defined(FEAT_TERMINAL)
+	if (popup_is_popup(curwin) && curbuf->b_term != NULL)
+	    // terminal in popup window: don't jump to another window
+	    return IN_OTHER_WIN;
+#endif
 	// Only change window focus when not clicking on or dragging the
 	// status line.  Do change focus when releasing the mouse button
 	// (MOUSE_FOCUS was set above if we dragged first).
--- a/src/optionstr.c
+++ b/src/optionstr.c
@@ -2128,6 +2128,12 @@ did_set_string_option(
 		errmsg = e_invarg;
 	}
     }
+    // 'wincolor'
+    else if (varp == &curwin->w_p_wcr)
+    {
+	if (curwin->w_buffer->b_term != NULL)
+	    term_update_colors();
+    }
 # if defined(MSWIN)
     // 'termwintype'
     else if (varp == &p_twt)
--- a/src/popupwin.c
+++ b/src/popupwin.c
@@ -1337,6 +1337,11 @@ popup_adjust_position(win_T *wp)
 
     wp->w_has_scrollbar = wp->w_want_scrollbar
 	   && (wp->w_topline > 1 || lnum <= wp->w_buffer->b_ml.ml_line_count);
+#ifdef FEAT_TERMINAL
+    if (wp->w_buffer->b_term != NULL)
+	// Terminal window never has a scrollbar, adjusts to window height.
+	wp->w_has_scrollbar = FALSE;
+#endif
     if (wp->w_has_scrollbar)
     {
 	++right_extra;
@@ -1769,20 +1774,13 @@ popup_create(typval_T *argvars, typval_T
 		semsg(_(e_nobufnr), argvars[0].vval.v_number);
 		return NULL;
 	    }
-#ifdef FEAT_TERMINAL
-	    if (buf->b_term != NULL)
-	    {
-		emsg(_("E278: Cannot put a terminal buffer in a popup window"));
-		return NULL;
-	    }
-#endif
 	}
 	else if (!(argvars[0].v_type == VAR_STRING
 			&& argvars[0].vval.v_string != NULL)
 		    && !(argvars[0].v_type == VAR_LIST
 			&& argvars[0].vval.v_list != NULL))
 	{
-	    emsg(_(e_listreq));
+	    emsg(_("E450: buffer number, text or a list required"));
 	    return NULL;
 	}
 	if (argvars[1].v_type != VAR_DICT || argvars[1].vval.v_dict == NULL)
@@ -2031,6 +2029,10 @@ popup_create(typval_T *argvars, typval_T
     redraw_all_later(NOT_VALID);
     popup_mask_refresh = TRUE;
 
+    // When running a terminal in the popup it becomes the current window.
+    if (buf->b_term != NULL)
+	win_enter(wp, FALSE);
+
     return wp;
 }
 
@@ -2107,6 +2109,13 @@ popup_close_and_callback(win_T *wp, typv
 {
     int id = wp->w_id;
 
+    if (wp == curwin && curbuf->b_term != NULL)
+    {
+	// Closing popup window with a terminal: put focus back on the previous
+	// window.
+	win_enter(prevwin, FALSE);
+    }
+
     // Just in case a check higher up is missing.
     if (wp == curwin && ERROR_IF_POPUP_WINDOW)
 	return;
@@ -2118,7 +2127,7 @@ popup_close_and_callback(win_T *wp, typv
     popup_close(id);
 }
 
-    static void
+    void
 popup_close_with_retval(win_T *wp, int retval)
 {
     typval_T res;
@@ -2834,7 +2843,10 @@ f_popup_getoptions(typval_T *argvars, ty
     int
 error_if_popup_window()
 {
-    if (WIN_IS_POPUP(curwin))
+    // win_execute() may set "curwin" to a popup window temporarily, but many
+    // commands are disallowed then.  When a terminal runs in the popup most
+    // things are allowed.
+    if (WIN_IS_POPUP(curwin) && curbuf->b_term == NULL)
     {
 	emsg(_("E994: Not allowed in a popup window"));
 	return TRUE;
@@ -2842,6 +2854,17 @@ error_if_popup_window()
     return FALSE;
 }
 
+    int
+error_if_term_popup_window()
+{
+    if (WIN_IS_POPUP(curwin) && curbuf->b_term != NULL)
+    {
+	emsg(_("E899: Not allowed for a terminal in a popup window"));
+	return TRUE;
+    }
+    return FALSE;
+}
+
 /*
  * Reset all the "handled_flag" flags in global popup windows and popup windows
  * in the current tab page.
@@ -2961,6 +2984,10 @@ popup_do_filter(int c)
     int		state;
     int		was_must_redraw = must_redraw;
 
+    // Popup window with terminal always gets focus.
+    if (popup_is_popup(curwin) && curbuf->b_term != NULL)
+	return FALSE;
+
     if (recursive)
 	return FALSE;
     recursive = TRUE;
@@ -3430,6 +3457,9 @@ update_popups(void (*win_update)(win_T *
 
 	wp->w_winrow -= top_off;
 	wp->w_wincol -= left_extra;
+	// cursor position matters in terminal
+	wp->w_wrow += top_off;
+	wp->w_wcol += left_extra;
 
 	total_width = popup_width(wp);
 	total_height = popup_height(wp);
--- a/src/proto/popupwin.pro
+++ b/src/proto/popupwin.pro
@@ -19,6 +19,7 @@ void f_popup_clear(typval_T *argvars, ty
 void f_popup_create(typval_T *argvars, typval_T *rettv);
 void f_popup_atcursor(typval_T *argvars, typval_T *rettv);
 void f_popup_beval(typval_T *argvars, typval_T *rettv);
+void popup_close_with_retval(win_T *wp, int retval);
 void popup_close_for_mouse_click(win_T *wp);
 void popup_handle_mouse_moved(void);
 void f_popup_filter_menu(typval_T *argvars, typval_T *rettv);
@@ -41,6 +42,7 @@ void f_popup_getpos(typval_T *argvars, t
 void f_popup_locate(typval_T *argvars, typval_T *rettv);
 void f_popup_getoptions(typval_T *argvars, typval_T *rettv);
 int error_if_popup_window(void);
+int error_if_term_popup_window(void);
 void popup_reset_handled(int handled_flag);
 win_T *find_next_popup(int lowest, int handled_flag);
 int popup_do_filter(int c);
--- a/src/proto/terminal.pro
+++ b/src/proto/terminal.pro
@@ -26,7 +26,8 @@ void term_update_window(win_T *wp);
 int term_is_finished(buf_T *buf);
 int term_show_buffer(buf_T *buf);
 void term_change_in_curbuf(void);
-int term_get_attr(buf_T *buf, linenr_T lnum, int col);
+int term_get_attr(win_T *wp, linenr_T lnum, int col);
+void term_update_colors(void);
 char_u *term_get_status_text(term_T *term);
 int set_ref_in_term(int copyID);
 void set_terminal_default_colors(int cterm_fg, int cterm_bg);
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -1158,9 +1158,17 @@ write_to_term(buf_T *buffer, char_u *msg
 term_send_mouse(VTerm *vterm, int button, int pressed)
 {
     VTermModifier   mod = VTERM_MOD_NONE;
-
-    vterm_mouse_move(vterm, mouse_row - W_WINROW(curwin),
-					    mouse_col - curwin->w_wincol, mod);
+    int		    row = mouse_row - W_WINROW(curwin);
+    int		    col = mouse_col - curwin->w_wincol;
+
+#ifdef FEAT_PROP_POPUP
+    if (popup_is_popup(curwin))
+    {
+	row -= popup_top_extra(curwin);
+	col -= popup_left_extra(curwin);
+    }
+#endif
+    vterm_mouse_move(vterm, row, col, mod);
     if (button != 0)
 	vterm_mouse_button(vterm, button, pressed, mod);
     return TRUE;
@@ -2027,20 +2035,32 @@ send_keys_to_term(term_T *term, int c, i
 	case K_MOUSEDOWN:
 	case K_MOUSELEFT:
 	case K_MOUSERIGHT:
-	    if (mouse_row < W_WINROW(curwin)
-		    || mouse_row >= (W_WINROW(curwin) + curwin->w_height)
-		    || mouse_col < curwin->w_wincol
-		    || mouse_col >= W_ENDCOL(curwin)
-		    || dragging_outside)
 	    {
-		// click or scroll outside the current window or on status line
-		// or vertical separator
-		if (typed)
+		int	row = mouse_row;
+		int	col = mouse_col;
+
+#ifdef FEAT_PROP_POPUP
+		if (popup_is_popup(curwin))
 		{
-		    stuffcharReadbuff(c);
-		    mouse_was_outside = TRUE;
+		    row -= popup_top_extra(curwin);
+		    col -= popup_left_extra(curwin);
 		}
-		return FAIL;
+#endif
+		if (row < W_WINROW(curwin)
+			|| row >= (W_WINROW(curwin) + curwin->w_height)
+			|| col < curwin->w_wincol
+			|| col >= W_ENDCOL(curwin)
+			|| dragging_outside)
+		{
+		    // click or scroll outside the current window or on status
+		    // line or vertical separator
+		    if (typed)
+		    {
+			stuffcharReadbuff(c);
+			mouse_was_outside = TRUE;
+		    }
+		    return FAIL;
+		}
 	    }
     }
     if (typed)
@@ -2057,10 +2077,17 @@ send_keys_to_term(term_T *term, int c, i
 }
 
     static void
-position_cursor(win_T *wp, VTermPos *pos)
+position_cursor(win_T *wp, VTermPos *pos, int add_off UNUSED)
 {
     wp->w_wrow = MIN(pos->row, MAX(0, wp->w_height - 1));
     wp->w_wcol = MIN(pos->col, MAX(0, wp->w_width - 1));
+#ifdef FEAT_PROP_POPUP
+    if (add_off && popup_is_popup(curwin))
+    {
+	wp->w_wrow += popup_top_extra(curwin);
+	wp->w_wcol += popup_left_extra(curwin);
+    }
+#endif
     wp->w_valid |= (VALID_WCOL|VALID_WROW);
 }
 
@@ -2361,7 +2388,7 @@ terminal_loop(int blocking)
 	if (termwinkey == Ctrl_W)
 	    termwinkey = 0;
     }
-    position_cursor(curwin, &curbuf->b_term->tl_cursor_pos);
+    position_cursor(curwin, &curbuf->b_term->tl_cursor_pos, TRUE);
     may_set_cursor_props(curbuf->b_term);
 
     while (blocking || vpeekc_nomap() != NUL)
@@ -2668,7 +2695,11 @@ hl2vtermAttr(int attr, cellattr_T *cell)
  * Convert the attributes of a vterm cell into an attribute index.
  */
     static int
-cell2attr(VTermScreenCellAttrs cellattrs, VTermColor cellfg, VTermColor cellbg)
+cell2attr(
+	win_T			*wp,
+	VTermScreenCellAttrs	cellattrs,
+	VTermColor		cellfg,
+	VTermColor		cellbg)
 {
     int attr = vtermAttr2hl(cellattrs);
 
@@ -2700,13 +2731,35 @@ cell2attr(VTermScreenCellAttrs cellattrs
 	int fg = color2index(&cellfg, TRUE, &bold);
 	int bg = color2index(&cellbg, FALSE, &bold);
 
-	// Use the "Terminal" highlighting for the default colors.
+	// Use the 'wincolor' or "Terminal" highlighting for the default
+	// colors.
 	if ((fg == 0 || bg == 0) && t_colors >= 16)
 	{
-	    if (fg == 0 && term_default_cterm_fg >= 0)
-		fg = term_default_cterm_fg + 1;
-	    if (bg == 0 && term_default_cterm_bg >= 0)
-		bg = term_default_cterm_bg + 1;
+	    int wincolor_fg = -1;
+	    int wincolor_bg = -1;
+
+	    if (wp != NULL && *wp->w_p_wcr != NUL)
+	    {
+		int id = syn_name2id(curwin->w_p_wcr);
+
+		// Get the 'wincolor' group colors.
+		if (id > 0)
+		    syn_id2cterm_bg(id, &wincolor_fg, &wincolor_bg);
+	    }
+	    if (fg == 0)
+	    {
+		if (wincolor_fg >= 0)
+		    fg = wincolor_fg + 1;
+		else if (term_default_cterm_fg >= 0)
+		    fg = term_default_cterm_fg + 1;
+	    }
+	    if (bg == 0)
+	    {
+		if (wincolor_bg >= 0)
+		    bg = wincolor_bg + 1;
+		else if (term_default_cterm_bg >= 0)
+		    bg = term_default_cterm_bg + 1;
+	    }
 	}
 
 	// with 8 colors set the bold attribute to get a bright foreground
@@ -2751,16 +2804,18 @@ term_scroll_up(term_T *term, int start_r
     VTermScreenCellAttrs attr;
     int			 clear_attr;
 
-    // Set the color to clear lines with.
-    vterm_state_get_default_colors(vterm_obtain_state(term->tl_vterm),
-								     &fg, &bg);
     vim_memset(&attr, 0, sizeof(attr));
-    clear_attr = cell2attr(attr, fg, bg);
 
     FOR_ALL_WINDOWS(wp)
     {
 	if (wp->w_buffer == term->tl_buffer)
+	{
+	    // Set the color to clear lines with.
+	    vterm_state_get_default_colors(vterm_obtain_state(term->tl_vterm),
+								     &fg, &bg);
+	    clear_attr = cell2attr(wp, attr, fg, bg);
 	    win_del_lines(wp, start_row, count, FALSE, FALSE, clear_attr);
+	}
     }
 }
 
@@ -2809,7 +2864,7 @@ handle_movecursor(
     FOR_ALL_WINDOWS(wp)
     {
 	if (wp->w_buffer == term->tl_buffer)
-	    position_cursor(wp, &pos);
+	    position_cursor(wp, &pos, FALSE);
     }
     if (term->tl_buffer == curbuf && !term->tl_normal_mode)
     {
@@ -3143,7 +3198,19 @@ term_after_channel_closed(term_T *term)
 	{
 	    aco_save_T	aco;
 	    int		do_set_w_closing = term->tl_buffer->b_nwindows == 0;
-
+#ifdef FEAT_PROP_POPUP
+	    win_T	*pwin = NULL;
+
+	    // If this was a terminal in a popup window, go back to the
+	    // previous window.
+	    if (popup_is_popup(curwin) && curbuf == term->tl_buffer)
+	    {
+		pwin = curwin;
+		if (win_valid(prevwin))
+		    win_enter(prevwin, FALSE);
+	    }
+	    else
+#endif
 	    // If this is the last normal window: exit Vim.
 	    if (term->tl_buffer->b_nwindows > 0 && only_one_window())
 	    {
@@ -3166,6 +3233,10 @@ term_after_channel_closed(term_T *term)
 	    if (do_set_w_closing)
 		curwin->w_closing = FALSE;
 	    aucmd_restbuf(&aco);
+#ifdef FEAT_PROP_POPUP
+	    if (pwin != NULL)
+		popup_close_with_retval(pwin, 0);
+#endif
 	    return TRUE;
 	}
 	if (term->tl_finish == TL_FINISH_OPEN
@@ -3277,7 +3348,11 @@ term_check_channel_closed_recently()
  * Advances "pos" to past the last column.
  */
     static void
-term_line2screenline(VTermScreen *screen, VTermPos *pos, int max_col)
+term_line2screenline(
+	win_T		*wp,
+	VTermScreen	*screen,
+	VTermPos	*pos,
+	int		max_col)
 {
     int off = screen_get_current_line_off();
 
@@ -3342,7 +3417,7 @@ term_line2screenline(VTermScreen *screen
 	    else
 		ScreenLines[off] = c;
 	}
-	ScreenAttrs[off] = cell2attr(cell.attrs, cell.fg, cell.bg);
+	ScreenAttrs[off] = cell2attr(wp, cell.attrs, cell.fg, cell.bg);
 
 	++pos->col;
 	++off;
@@ -3393,7 +3468,7 @@ update_system_term(term_T *term)
 	{
 	    int max_col = MIN(Columns, term->tl_cols);
 
-	    term_line2screenline(screen, &pos, max_col);
+	    term_line2screenline(NULL, screen, &pos, max_col);
 	}
 	else
 	    pos.col = 0;
@@ -3462,15 +3537,20 @@ term_update_window(win_T *wp)
 
     newrows = 99999;
     newcols = 99999;
-    FOR_ALL_WINDOWS(twp)
-    {
+    for (twp = firstwin; ; twp = twp->w_next)
+    {
+	// Always use curwin, it may be a popup window.
+	win_T *wwp = twp == NULL ? curwin : twp;
+
 	// When more than one window shows the same terminal, use the
 	// smallest size.
-	if (twp->w_buffer == term->tl_buffer)
+	if (wwp->w_buffer == term->tl_buffer)
 	{
-	    newrows = MIN(newrows, twp->w_height);
-	    newcols = MIN(newcols, twp->w_width);
+	    newrows = MIN(newrows, wwp->w_height);
+	    newcols = MIN(newcols, wwp->w_width);
 	}
+	if (twp == NULL)
+	    break;
     }
     if (newrows == 99999 || newcols == 99999)
 	return; // safety exit
@@ -3493,7 +3573,7 @@ term_update_window(win_T *wp)
 
     // The cursor may have been moved when resizing.
     vterm_state_get_cursorpos(state, &pos);
-    position_cursor(wp, &pos);
+    position_cursor(wp, &pos, FALSE);
 
     for (pos.row = term->tl_dirty_row_start; pos.row < term->tl_dirty_row_end
 					  && pos.row < wp->w_height; ++pos.row)
@@ -3502,7 +3582,7 @@ term_update_window(win_T *wp)
 	{
 	    int max_col = MIN(wp->w_width, term->tl_cols);
 
-	    term_line2screenline(screen, &pos, max_col);
+	    term_line2screenline(wp, screen, &pos, max_col);
 	}
 	else
 	    pos.col = 0;
@@ -3511,7 +3591,11 @@ term_update_window(win_T *wp)
 #ifdef FEAT_MENU
 				+ winbar_height(wp)
 #endif
-				, wp->w_wincol, pos.col, wp->w_width, 0);
+				, wp->w_wincol, pos.col, wp->w_width,
+#ifdef FEAT_PROP_POPUP
+				popup_is_popup(wp) ? SLF_POPUP :
+#endif
+				0);
     }
     term->tl_dirty_row_start = MAX_ROW;
     term->tl_dirty_row_end = 0;
@@ -3564,8 +3648,9 @@ term_change_in_curbuf(void)
  * Use a negative "col" to get the filler background color.
  */
     int
-term_get_attr(buf_T *buf, linenr_T lnum, int col)
-{
+term_get_attr(win_T *wp, linenr_T lnum, int col)
+{
+    buf_T	*buf = wp->w_buffer;
     term_T	*term = buf->b_term;
     sb_line_T	*line;
     cellattr_T	*cellattr;
@@ -3580,7 +3665,7 @@ term_get_attr(buf_T *buf, linenr_T lnum,
 	else
 	    cellattr = line->sb_cells + col;
     }
-    return cell2attr(cellattr->attrs, cellattr->fg, cellattr->bg);
+    return cell2attr(wp, cellattr->attrs, cellattr->fg, cellattr->bg);
 }
 
 /*
@@ -3597,7 +3682,7 @@ cterm_color2vterm(int nr, VTermColor *rg
  * Initialize term->tl_default_color from the environment.
  */
     static void
-init_default_colors(term_T *term)
+init_default_colors(term_T *term, win_T *wp)
 {
     VTermColor	    *fg, *bg;
     int		    fgval, bgval;
@@ -3624,8 +3709,11 @@ init_default_colors(term_T *term)
     bg->red = bg->green = bg->blue = bgval;
     fg->ansi_index = bg->ansi_index = VTERM_ANSI_INDEX_DEFAULT;
 
-    // The "Terminal" highlight group overrules the defaults.
-    id = syn_name2id((char_u *)"Terminal");
+    // The 'wincolor' or "Terminal" highlight group overrules the defaults.
+    if (wp != NULL && *wp->w_p_wcr != NUL)
+	id = syn_name2id(wp->w_p_wcr);
+    else
+	id = syn_name2id((char_u *)"Terminal");
 
     // Use the actual color for the GUI and when 'termguicolors' is set.
 #if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
@@ -4122,7 +4210,7 @@ create_vterm(term_T *term, int rows, int
     // TODO: depends on 'encoding'.
     vterm_set_utf8(vterm, 1);
 
-    init_default_colors(term);
+    init_default_colors(term, NULL);
 
     vterm_state_set_default_colors(
 	    state,
@@ -4158,6 +4246,21 @@ create_vterm(term_T *term, int rows, int
 }
 
 /*
+ * Called when 'wincolor' was set.
+ */
+    void
+term_update_colors(void)
+{
+    term_T *term = curwin->w_buffer->b_term;
+
+    init_default_colors(term, curwin);
+    vterm_state_set_default_colors(
+	    vterm_obtain_state(term->tl_vterm),
+	    &term->tl_default_color.fg,
+	    &term->tl_default_color.bg);
+}
+
+/*
  * Return the text to show for the buffer name and status.
  */
     char_u *
@@ -4857,7 +4960,7 @@ term_load_dump(typval_T *argvars, typval
 	VTermPos	cursor_pos1;
 	VTermPos	cursor_pos2;
 
-	init_default_colors(term);
+	init_default_colors(term, NULL);
 
 	rettv->vval.v_number = buf->b_fnum;
 
@@ -5571,7 +5674,7 @@ f_term_scrape(typval_T *argvars, typval_
 				     bg.red, bg.green, bg.blue);
 	dict_add_string(dcell, "bg", rgb);
 
-	dict_add_number(dcell, "attr", cell2attr(attrs, fg, bg));
+	dict_add_number(dcell, "attr", cell2attr(NULL, attrs, fg, bg));
 	dict_add_number(dcell, "width", width);
 
 	++pos.col;
new file mode 100644
--- /dev/null
+++ b/src/testdir/dumps/Test_terminal_popup_1.dump
@@ -0,0 +1,15 @@
+|0+0&#ffffff0| @73
+|1| @73
+|2| @73
+|3| @12|╔+0#0000001#ffd7ff255|═@44|╗| +0#0000000#ffffff0@13
+|4| @12|║+0#0000001#ffd7ff255>s+0#0000000#ffffff0|o|m|e| |t|e|x|t| @35|║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|5| @12|║+0#0000001#ffd7ff255|t+0#0000000#ffffff0|o| |e|d|i|t| @37|║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|6| @12|║+0#0000001#ffd7ff255|i+0#0000000#ffffff0|n| |a| |p|o|p|u|p| |w|i|n|d|o|w| @27|║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|7| @12|║+0#0000001#ffd7ff255|~+0#4040ff13#ffffff0| @43|║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|8| @12|║+0#0000001#ffd7ff255|~+0#4040ff13#ffffff0| @43|║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|9| @12|║+0#0000001#ffd7ff255|~+0#4040ff13#ffffff0| @43|║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|1|0| @11|║+0#0000001#ffd7ff255|"+0#0000000#ffffff0|X|t|e|x|t|"| |3|L|,| |3|6|C| @11|1|,|1| @10|A|l@1| |║+0#0000001#ffd7ff255| +0#0000000#ffffff0@13
+|1@1| @11|╚+0#0000001#ffd7ff255|═@44|⇲| +0#0000000#ffffff0@13
+|1|2| @72
+|1|3| @72
+@57|0|,|0|-|1| @8|A|l@1| 
new file mode 100644
--- /dev/null
+++ b/src/testdir/dumps/Test_terminal_popup_2.dump
@@ -0,0 +1,15 @@
+>0+0&#ffffff0| @73
+|1| @73
+|2| @73
+|3| @73
+|4| @73
+|5| @73
+|6| @73
+|7| @73
+|8| @73
+|9| @73
+|1|0| @72
+|1@1| @72
+|1|2| @72
+|1|3| @72
+|"|[|N|o| |N|a|m|e|]|"| |[|M|o|d|i|f|i|e|d|]| |2|0| |l|i|n|e|s| |-@1|5|%|-@1| @18|1|,|1| @10|T|o|p| 
new file mode 100644
--- /dev/null
+++ b/src/testdir/dumps/Test_terminal_popup_3.dump
@@ -0,0 +1,15 @@
+|0+0&#ffffff0| @73
+|1| @73
+|2| @73
+|3| @12|╔+0&#a8a8a8255|═@44|╗| +0&#ffffff0@13
+|4| @12|║+0&#a8a8a8255|s|o|m|e| |t|e|x|t| @35|║| +0&#ffffff0@13
+|5| @12|║+0&#a8a8a8255|t|o| >e+0&#ffff4012|d|i|t| +0&#a8a8a8255@37|║| +0&#ffffff0@13
+|6| @12|║+0&#a8a8a8255|i|n| |a| |p|o|p|u|p| |w|i|n|d|o|w| @27|║| +0&#ffffff0@13
+|7| @12|║+0&#a8a8a8255|~+0#4040ff13&| @43|║+0#0000000&| +0&#ffffff0@13
+|8| @12|║+0&#a8a8a8255|~+0#4040ff13&| @43|║+0#0000000&| +0&#ffffff0@13
+|9| @12|║+0&#a8a8a8255|~+0#4040ff13&| @43|║+0#0000000&| +0&#ffffff0@13
+|1|0| @11|║+0&#a8a8a8255|/|e|d|i|t| @21|2|,|4| @10|A|l@1| |║| +0&#ffffff0@13
+|1@1| @11|╚+0&#a8a8a8255|═@44|⇲| +0&#ffffff0@13
+|1|2| @72
+|1|3| @72
+@57|0|,|0|-|1| @8|A|l@1| 
--- a/src/testdir/test_terminal.vim
+++ b/src/testdir/test_terminal.vim
@@ -2321,3 +2321,44 @@ func Test_terminal_api_arg()
   unlet! g:called_bufnum
   unlet! g:called_arg
 endfunc
+
+func Test_terminal_in_popup()
+  CheckRunVimInTerminal
+
+  let text =<< trim END
+    some text
+    to edit
+    in a popup window
+  END
+  call writefile(text, 'Xtext')
+  let cmd = GetVimCommandClean()
+  let lines = [
+	\ 'call setline(1, range(20))',
+	\ 'hi PopTerm ctermbg=grey',
+	\ 'func OpenTerm(setColor)',
+	\ "  let buf = term_start('" .. cmd .. " Xtext', #{hidden: 1, term_finish: 'close'})",
+	\ '  let winid = popup_create(buf, #{minwidth: 45, minheight: 7, border: [], drag: 1, resize: 1})',
+	\ '  if a:setColor',
+	\ '    call win_execute(winid, "set wincolor=PopTerm")',
+	\ '  endif',
+	\ 'endfunc',
+	\ 'call OpenTerm(0)',
+	\ ]
+  call writefile(lines, 'XtermPopup')
+  let buf = RunVimInTerminal('-S XtermPopup', #{rows: 15})
+  call VerifyScreenDump(buf, 'Test_terminal_popup_1', {})
+
+  call term_sendkeys(buf, ":q\<CR>")
+  call VerifyScreenDump(buf, 'Test_terminal_popup_2', {})
+ 
+  call term_sendkeys(buf, ":call OpenTerm(1)\<CR>")
+  call term_sendkeys(buf, ":set hlsearch\<CR>")
+  call term_sendkeys(buf, "/edit\<CR>")
+  call VerifyScreenDump(buf, 'Test_terminal_popup_3', {})
+ 
+  call term_sendkeys(buf, ":q\<CR>")
+  call term_wait(buf, 50)  " wait for terminal to vanish
+
+  call StopVimInTerminal(buf)
+  call delete('XtermPopup')
+endfunc
--- a/src/version.c
+++ b/src/version.c
@@ -743,6 +743,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    191,
+/**/
     190,
 /**/
     189,
--- a/src/window.c
+++ b/src/window.c
@@ -4344,7 +4344,7 @@ win_goto(win_T *wp)
 #endif
 
 #ifdef FEAT_PROP_POPUP
-    if (ERROR_IF_POPUP_WINDOW)
+    if (ERROR_IF_POPUP_WINDOW || ERROR_IF_TERM_POPUP_WINDOW)
 	return;
     if (popup_is_popup(wp))
     {
@@ -4486,6 +4486,10 @@ win_goto_ver(
 {
     win_T	*win;
 
+#ifdef FEAT_PROP_POPUP
+    if (ERROR_IF_TERM_POPUP_WINDOW)
+	return;
+#endif
     win = win_vert_neighbor(curtab, curwin, up, count);
     if (win != NULL)
 	win_goto(win);
@@ -4564,6 +4568,10 @@ win_goto_hor(
 {
     win_T	*win;
 
+#ifdef FEAT_PROP_POPUP
+    if (ERROR_IF_TERM_POPUP_WINDOW)
+	return;
+#endif
     win = win_horz_neighbor(curtab, curwin, left, count);
     if (win != NULL)
 	win_goto(win);