patch 8.1.0228: dropping files is ignored while Vim is busy

Problem:    Dropping files is ignored while Vim is busy.
Solution:   Postpone the effect of dropping files until it's safe.

src/ex_docmd.c

@@ -7859,57 +7859,37 @@ ex_shell(exarg_T *eap UNUSED)
     do_shell(NULL, 0);
 }
 
-#if defined(HAVE_DROP_FILE) \
+#if defined(HAVE_DROP_FILE) || defined(PROTO)
-	|| (defined(FEAT_GUI_GTK) && defined(FEAT_DND)) \
-	|| defined(FEAT_GUI_MSWIN) \
-	|| defined(FEAT_GUI_MAC) \
-	|| defined(PROTO)
 
-/*
+static int drop_busy = FALSE;
- * Handle a file drop. The code is here because a drop is *nearly* like an
+static int drop_filec;
- * :args command, but not quite (we have a list of exact filenames, so we
+static char_u **drop_filev = NULL;
- * don't want to (a) parse a command line, or (b) expand wildcards. So the
+static int drop_split;
- * code is very similar to :args and hence needs access to a lot of the static
+static void (*drop_callback)(void *);
- * functions in this file.
+static void *drop_cookie;
- *
+
- * The list should be allocated using alloc(), as should each item in the
+    static void
- * list. This function takes over responsibility for freeing the list.
+handle_drop_internal(void)
- *
- * XXX The list is made into the argument list. This is freed using
- * FreeWild(), which does a series of vim_free() calls.
- */
-    void
-handle_drop(
-    int		filec,		/* the number of files dropped */
-    char_u	**filev,	/* the list of files dropped */
-    int		split)		/* force splitting the window */
 {
     exarg_T	ea;
     int		save_msg_scroll = msg_scroll;
 
-    /* Postpone this while editing the command line. */
+    // Setting the argument list may cause screen updates and being called
-    if (text_locked())
+    // recursively.  Avoid that by setting drop_busy.
-	return;
+    drop_busy = TRUE;
-    if (curbuf_locked())
-	return;
-
-    /* When the screen is being updated we should not change buffers and
-     * windows structures, it may cause freed memory to be used. */
-    if (updating_screen)
-	return;
 
     /* Check whether the current buffer is changed. If so, we will need
      * to split the current window or data could be lost.
      * We don't need to check if the 'hidden' option is set, as in this
      * case the buffer won't be lost.
      */
-    if (!buf_hide(curbuf) && !split)
+    if (!buf_hide(curbuf) && !drop_split)
     {
 	++emsg_off;
-	split = check_changed(curbuf, CCGD_AW);
+	drop_split = check_changed(curbuf, CCGD_AW);
 	--emsg_off;
     }
-    if (split)
+    if (drop_split)
     {
 	if (win_split(0, 0) == FAIL)
 	    return;
@@ -7924,7 +7904,7 @@ handle_drop(
     /*
      * Set up the new argument list.
      */
-    alist_set(ALIST(curwin), filec, filev, FALSE, NULL, 0);
+    alist_set(ALIST(curwin), drop_filec, drop_filev, FALSE, NULL, 0);
 
     /*
      * Move to the first file.
@@ -7942,6 +7922,78 @@ handle_drop(
      * unexpectedly.  The screen will be redrawn by the caller, thus
      * msg_scroll being set by displaying a message is irrelevant. */
     msg_scroll = save_msg_scroll;
+
+    if (drop_callback != NULL)
+	drop_callback(drop_cookie);
+
+    drop_filev = NULL;
+    drop_busy = FALSE;
+}
+
+/*
+ * Handle a file drop. The code is here because a drop is *nearly* like an
+ * :args command, but not quite (we have a list of exact filenames, so we
+ * don't want to (a) parse a command line, or (b) expand wildcards. So the
+ * code is very similar to :args and hence needs access to a lot of the static
+ * functions in this file.
+ *
+ * The "filev" list must have been allocated using alloc(), as should each item
+ * in the list. This function takes over responsibility for freeing the "filev"
+ * list.
+ */
+    void
+handle_drop(
+    int		filec,		// the number of files dropped
+    char_u	**filev,	// the list of files dropped
+    int		split,		// force splitting the window
+    void	(*callback)(void *), // to be called after setting the argument
+				     // list
+    void	*cookie)	// argument for "callback" (allocated)
+{
+    // Cannot handle recursive drops, finish the pending one.
+    if (drop_busy)
+    {
+	FreeWild(filec, filev);
+	vim_free(cookie);
+	return;
+    }
+
+    // When calling handle_drop() more than once in a row we only use the last
+    // one.
+    if (drop_filev != NULL)
+    {
+	FreeWild(drop_filec, drop_filev);
+	vim_free(drop_cookie);
+    }
+
+    drop_filec = filec;
+    drop_filev = filev;
+    drop_split = split;
+    drop_callback = callback;
+    drop_cookie = cookie;
+
+    // Postpone this when:
+    // - editing the command line
+    // - not possible to change the current buffer
+    // - updating the screen
+    // As it may change buffers and window structures that are in use and cause
+    // freed memory to be used.
+    if (text_locked() || curbuf_locked() || updating_screen)
+	return;
+
+    handle_drop_internal();
+}
+
+/*
+ * To be called when text is unlocked, curbuf is unlocked or updating_screen is
+ * reset: Handle a postponed drop.
+ */
+    void
+handle_any_postponed_drop(void)
+{
+    if (!drop_busy && drop_filev != NULL
+		     && !text_locked() && !curbuf_locked() && !updating_screen)
+	handle_drop_internal();
 }
 #endif
 

src/gui.c

@@ -5383,10 +5383,7 @@ gui_do_findrepl(
 
 #endif
 
-#if (defined(FEAT_DND) && defined(FEAT_GUI_GTK)) \
+#if defined(HAVE_DROP_FILE) || defined(PROTO)
-	|| defined(FEAT_GUI_MSWIN) \
-	|| defined(FEAT_GUI_MAC) \
-	|| defined(PROTO)
 
 static void gui_wingoto_xy(int x, int y);
 
@@ -5408,6 +5405,42 @@ gui_wingoto_xy(int x, int y)
     }
 }
 
+/*
+ * Function passed to handle_drop() for the actions to be done after the
+ * argument list has been updated.
+ */
+    static void
+drop_callback(void *cookie)
+{
+    char_u	*p = cookie;
+
+    /* If Shift held down, change to first file's directory.  If the first
+     * item is a directory, change to that directory (and let the explorer
+     * plugin show the contents). */
+    if (p != NULL)
+    {
+	if (mch_isdir(p))
+	{
+	    if (mch_chdir((char *)p) == 0)
+		shorten_fnames(TRUE);
+	}
+	else if (vim_chdirfile(p, "drop") == OK)
+	    shorten_fnames(TRUE);
+	vim_free(p);
+    }
+
+    /* Update the screen display */
+    update_screen(NOT_VALID);
+# ifdef FEAT_MENU
+    gui_update_menus(0);
+# endif
+#ifdef FEAT_TITLE
+    maketitle();
+#endif
+    setcursor();
+    out_flush_cursor(FALSE, FALSE);
+}
+
 /*
  * Process file drop.  Mouse cursor position, key modifiers, name of files
  * and count of files are given.  Argument "fnames[count]" has full pathnames
@@ -5488,33 +5521,8 @@ gui_handle_drop(
 	    vim_free(fnames);
 	}
 	else
-	    handle_drop(count, fnames, (modifiers & MOUSE_CTRL) != 0);
+	    handle_drop(count, fnames, (modifiers & MOUSE_CTRL) != 0,
-
+		    drop_callback, (void *)p);
-	/* If Shift held down, change to first file's directory.  If the first
-	 * item is a directory, change to that directory (and let the explorer
-	 * plugin show the contents). */
-	if (p != NULL)
-	{
-	    if (mch_isdir(p))
-	    {
-		if (mch_chdir((char *)p) == 0)
-		    shorten_fnames(TRUE);
-	    }
-	    else if (vim_chdirfile(p, "drop") == OK)
-		shorten_fnames(TRUE);
-	    vim_free(p);
-	}
-
-	/* Update the screen display */
-	update_screen(NOT_VALID);
-# ifdef FEAT_MENU
-	gui_update_menus(0);
-# endif
-#ifdef FEAT_TITLE
-	maketitle();
-#endif
-	setcursor();
-	out_flush_cursor(FALSE, FALSE);
     }
 
     entered = FALSE;

src/gui.h

@@ -65,8 +65,9 @@
 /*
  * GUIs that support dropping files on a running Vim.
  */
-#if defined(FEAT_GUI_MSWIN) || defined(FEAT_GUI_MAC) \
+#if (defined(FEAT_DND) && defined(FEAT_GUI_GTK)) \
-	|| defined(FEAT_GUI_GTK)
+	|| defined(FEAT_GUI_MSWIN) \
+	|| defined(FEAT_GUI_MAC)
 # define HAVE_DROP_FILE
 #endif
 

src/gui_mac.c

@@ -1007,6 +1007,55 @@ struct SelectionRange /* for handling kCoreClassEvent:kOpenDocuments:keyAEPositi
     long theDate; // modification date/time
 };
 
+static long drop_numFiles;
+static short drop_gotPosition;
+static SelectionRange drop_thePosition;
+
+    static void
+drop_callback(void *cookie UNUSED)
+{
+    /* TODO: Handle the goto/select line more cleanly */
+    if ((drop_numFiles == 1) & (drop_gotPosition))
+    {
+	if (drop_thePosition.lineNum >= 0)
+	{
+	    lnum = drop_thePosition.lineNum + 1;
+	/*  oap->motion_type = MLINE;
+	    setpcmark();*/
+	    if (lnum < 1L)
+		lnum = 1L;
+	    else if (lnum > curbuf->b_ml.ml_line_count)
+		lnum = curbuf->b_ml.ml_line_count;
+	    curwin->w_cursor.lnum = lnum;
+	    curwin->w_cursor.col = 0;
+	/*  beginline(BL_SOL | BL_FIX);*/
+	}
+	else
+	    goto_byte(drop_thePosition.startRange + 1);
+    }
+
+    /* Update the screen display */
+    update_screen(NOT_VALID);
+
+    /* Select the text if possible */
+    if (drop_gotPosition)
+    {
+	VIsual_active = TRUE;
+	VIsual_select = FALSE;
+	VIsual = curwin->w_cursor;
+	if (drop_thePosition.lineNum < 0)
+	{
+	    VIsual_mode = 'v';
+	    goto_byte(drop_thePosition.endRange);
+	}
+	else
+	{
+	    VIsual_mode = 'V';
+	    VIsual.col = 0;
+	}
+    }
+}
+
 /* The IDE uses the optional keyAEPosition parameter to tell the ed-
    itor the selection range. If lineNum is zero or greater, scroll the text
    to the specified line. If lineNum is less than zero, use the values in
@@ -1113,48 +1162,10 @@ HandleODocAE(const AppleEvent *theAEvent, AppleEvent *theReply, long refCon)
     }
 
     /* Handle the drop, :edit to get to the file */
-    handle_drop(numFiles, fnames, FALSE);
+    drop_numFiles = numFiles;
-
+    drop_gotPosition = gotPosition;
-    /* TODO: Handle the goto/select line more cleanly */
+    drop_thePosition = thePosition;
-    if ((numFiles == 1) & (gotPosition))
+    handle_drop(numFiles, fnames, FALSE, drop_callback, NULL);
-    {
-	if (thePosition.lineNum >= 0)
-	{
-	    lnum = thePosition.lineNum + 1;
-	/*  oap->motion_type = MLINE;
-	    setpcmark();*/
-	    if (lnum < 1L)
-		lnum = 1L;
-	    else if (lnum > curbuf->b_ml.ml_line_count)
-		lnum = curbuf->b_ml.ml_line_count;
-	    curwin->w_cursor.lnum = lnum;
-	    curwin->w_cursor.col = 0;
-	/*  beginline(BL_SOL | BL_FIX);*/
-	}
-	else
-	    goto_byte(thePosition.startRange + 1);
-    }
-
-    /* Update the screen display */
-    update_screen(NOT_VALID);
-
-    /* Select the text if possible */
-    if (gotPosition)
-    {
-	VIsual_active = TRUE;
-	VIsual_select = FALSE;
-	VIsual = curwin->w_cursor;
-	if (thePosition.lineNum < 0)
-	{
-	    VIsual_mode = 'v';
-	    goto_byte(thePosition.endRange);
-	}
-	else
-	{
-	    VIsual_mode = 'V';
-	    VIsual.col = 0;
-	}
-    }
 
     setcursor();
     out_flush();

src/main.c

@@ -911,7 +911,7 @@ vim_main2(void)
 
     /*
      * Call the main command loop.  This never returns.
-    */
+     */
     main_loop(FALSE, FALSE);
 
 #endif /* NO_VIM_MAIN */
@@ -1155,9 +1155,15 @@ main_loop(
 	else if (do_redraw || stuff_empty())
 	{
 #ifdef FEAT_GUI
-	    /* If ui_breakcheck() was used a resize may have been postponed. */
+	    // If ui_breakcheck() was used a resize may have been postponed.
 	    gui_may_resize_shell();
 #endif
+#ifdef HAVE_DROP_FILE
+	    // If files were dropped while text was locked or the curbuf was
+	    // locked, this would be a good time to handle the drop.
+	    handle_any_postponed_drop();
+#endif
+
 	    /* Trigger CursorMoved if the cursor moved. */
 	    if (!finish_op && (
 			has_cursormoved()

src/proto/ex_docmd.pro

@@ -31,7 +31,8 @@ void not_exiting(void);
 void tabpage_close(int forceit);
 void tabpage_close_other(tabpage_T *tp, int forceit);
 void ex_all(exarg_T *eap);
-void handle_drop(int filec, char_u **filev, int split);
+void handle_drop(int filec, char_u **filev, int split, void (*callback)(void *), void *cookie);
+void handle_any_postponed_drop(void);
 void alist_clear(alist_T *al);
 void alist_init(alist_T *al);
 void alist_unlink(alist_T *al);

src/screen.c

@@ -526,6 +526,12 @@ reset_updating_screen(int may_resize_shell UNUSED)
 #ifdef FEAT_TERMINAL
     term_check_channel_closed_recently();
 #endif
+
+#ifdef HAVE_DROP_FILE
+    // If handle_drop() was called while updating_screen was TRUE need to
+    // handle the drop now.
+    handle_any_postponed_drop();
+#endif
 }
 
 /*

src/version.c

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