patch 8.1.1231: asking about existing swap file unnecessarily

Problem:    Asking about existing swap file unnecessarily.
Solution:   When it is safe, delete the swap file.  Remove
            HAS_SWAP_EXISTS_ACTION, it is always defined. (closes #1237)

runtime/doc/usr_11.txt

@@ -1,4 +1,4 @@
-*usr_11.txt*	For Vim version 8.1.  Last change: 2019 Feb 04
+*usr_11.txt*	For Vim version 8.1.  Last change: 2019 Apr 28
 
 		     VIM USER MANUAL - by Bram Moolenaar
 
@@ -205,6 +205,13 @@ something wrong.  It may be one of these two situations.
       NEWER than swap file! ~
 
 
+NOTE that in the following situation Vim knows the swap file is not useful and
+will automatically delete it:
+- The file is a valid swap file (Magic number is correct).
+- The flag that the file was modified is not set.
+- The process is not running.
+
+
 UNREADABLE SWAP FILE
 
 Sometimes the line

src/buffer.c

@@ -972,43 +972,38 @@ goto_buffer(
     int		dir,
     int		count)
 {
-#if defined(HAS_SWAP_EXISTS_ACTION)
     bufref_T	old_curbuf;
 
     set_bufref(&old_curbuf, curbuf);
 
     swap_exists_action = SEA_DIALOG;
-#endif
     (void)do_buffer(*eap->cmd == 's' ? DOBUF_SPLIT : DOBUF_GOTO,
 					     start, dir, count, eap->forceit);
-#if defined(HAS_SWAP_EXISTS_ACTION)
     if (swap_exists_action == SEA_QUIT && *eap->cmd == 's')
     {
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 	cleanup_T   cs;
 
 	/* Reset the error/interrupt/exception state here so that
 	 * aborting() returns FALSE when closing a window. */
 	enter_cleanup(&cs);
-# endif
+#endif
 
 	/* Quitting means closing the split window, nothing else. */
 	win_close(curwin, TRUE);
 	swap_exists_action = SEA_NONE;
 	swap_exists_did_quit = TRUE;
 
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 	/* Restore the error/interrupt/exception state if not discarded by a
 	 * new aborting error, interrupt, or uncaught exception. */
 	leave_cleanup(&cs);
-# endif
+#endif
     }
     else
 	handle_swap_exists(&old_curbuf);
-#endif
 }
 
-#if defined(HAS_SWAP_EXISTS_ACTION) || defined(PROTO)
 /*
  * Handle the situation of swap_exists_action being set.
  * It is allowed for "old_curbuf" to be NULL or invalid.
@@ -1016,21 +1011,21 @@ goto_buffer(
     void
 handle_swap_exists(bufref_T *old_curbuf)
 {
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
     cleanup_T	cs;
-# endif
+#endif
-# ifdef FEAT_SYN_HL
+#ifdef FEAT_SYN_HL
     long	old_tw = curbuf->b_p_tw;
-# endif
+#endif
     buf_T	*buf;
 
     if (swap_exists_action == SEA_QUIT)
     {
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 	/* Reset the error/interrupt/exception state here so that
 	 * aborting() returns FALSE when closing a buffer. */
 	enter_cleanup(&cs);
-# endif
+#endif
 
 	/* User selected Quit at ATTENTION prompt.  Go back to previous
 	 * buffer.  If that buffer is gone or the same as the current one,
@@ -1053,26 +1048,26 @@ handle_swap_exists(bufref_T *old_curbuf)
 	    // restore msg_silent, so that the command line will be shown
 	    msg_silent = old_msg_silent;
 
-# ifdef FEAT_SYN_HL
+#ifdef FEAT_SYN_HL
 	    if (old_tw != curbuf->b_p_tw)
 		check_colorcolumn(curwin);
-# endif
+#endif
 	}
 	/* If "old_curbuf" is NULL we are in big trouble here... */
 
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 	/* Restore the error/interrupt/exception state if not discarded by a
 	 * new aborting error, interrupt, or uncaught exception. */
 	leave_cleanup(&cs);
-# endif
+#endif
     }
     else if (swap_exists_action == SEA_RECOVER)
     {
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 	/* Reset the error/interrupt/exception state here so that
 	 * aborting() returns FALSE when closing a buffer. */
 	enter_cleanup(&cs);
-# endif
+#endif
 
 	/* User selected Recover at ATTENTION prompt. */
 	msg_scroll = TRUE;
@@ -1081,15 +1076,14 @@ handle_swap_exists(bufref_T *old_curbuf)
 	cmdline_row = msg_row;
 	do_modelines(0);
 
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 	/* Restore the error/interrupt/exception state if not discarded by a
 	 * new aborting error, interrupt, or uncaught exception. */
 	leave_cleanup(&cs);
-# endif
+#endif
     }
     swap_exists_action = SEA_NONE;
 }
-#endif
 
 /*
  * do_bufdel() - delete or unload buffer(s)
@@ -5259,28 +5253,23 @@ ex_buffer_all(exarg_T *eap)
 		continue;
 
 	    /* Open the buffer in this window. */
-#if defined(HAS_SWAP_EXISTS_ACTION)
 	    swap_exists_action = SEA_DIALOG;
-#endif
 	    set_curbuf(buf, DOBUF_GOTO);
 	    if (!bufref_valid(&bufref))
 	    {
 		/* autocommands deleted the buffer!!! */
-#if defined(HAS_SWAP_EXISTS_ACTION)
 		swap_exists_action = SEA_NONE;
-#endif
 		break;
 	    }
-#if defined(HAS_SWAP_EXISTS_ACTION)
 	    if (swap_exists_action == SEA_QUIT)
 	    {
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 		cleanup_T   cs;
 
 		/* Reset the error/interrupt/exception state here so that
 		 * aborting() returns FALSE when closing a window. */
 		enter_cleanup(&cs);
-# endif
+#endif
 
 		/* User selected Quit at ATTENTION prompt; close this window. */
 		win_close(curwin, TRUE);
@@ -5288,16 +5277,15 @@ ex_buffer_all(exarg_T *eap)
 		swap_exists_action = SEA_NONE;
 		swap_exists_did_quit = TRUE;
 
-# if defined(FEAT_EVAL)
+#if defined(FEAT_EVAL)
 		/* Restore the error/interrupt/exception state if not
 		 * discarded by a new aborting error, interrupt, or uncaught
 		 * exception. */
 		leave_cleanup(&cs);
-# endif
+#endif
 	    }
 	    else
 		handle_swap_exists(NULL);
-#endif
 	}
 
 	ui_breakcheck();

src/ex_cmds.c

@@ -4258,9 +4258,7 @@ do_ecmd(
 	topline = curwin->w_topline;
 	if (!oldbuf)			    /* need to read the file */
 	{
-#if defined(HAS_SWAP_EXISTS_ACTION)
 	    swap_exists_action = SEA_DIALOG;
-#endif
 	    curbuf->b_flags |= BF_CHECK_RO; /* set/reset 'ro' flag */
 
 	    /*
@@ -4273,11 +4271,9 @@ do_ecmd(
 	    (void)open_buffer(FALSE, eap, readfile_flags);
 #endif
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
 	    if (swap_exists_action == SEA_QUIT)
 		retval = FAIL;
 	    handle_swap_exists(&old_curbuf);
-#endif
 	}
 	else
 	{

src/fileio.c

@@ -684,15 +684,13 @@ readfile(
 #endif
     }
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
+    // If "Quit" selected at ATTENTION dialog, don't load the file
-    /* If "Quit" selected at ATTENTION dialog, don't load the file */
     if (swap_exists_action == SEA_QUIT)
     {
 	if (!read_buffer && !read_stdin)
 	    close(fd);
 	return FAIL;
     }
-#endif
 
     ++no_wait_return;	    /* don't wait for return yet */
 

src/globals.h

@@ -966,7 +966,6 @@ EXTERN int	emsg_silent INIT(= 0);	/* don't print error messages */
 EXTERN int	emsg_noredir INIT(= 0);	/* don't redirect error messages */
 EXTERN int	cmd_silent INIT(= FALSE); /* don't echo the command line */
 
-# define HAS_SWAP_EXISTS_ACTION
 EXTERN int	swap_exists_action INIT(= SEA_NONE);
 					/* For dialog when swap file already
 					 * exists. */
@@ -1644,6 +1643,9 @@ EXTERN int *eval_lavars_used INIT(= NULL);
 #endif
 
 #ifdef MSWIN
+# ifdef PROTO
+typedef int HINSTANCE;
+# endif
 EXTERN int ctrl_break_was_pressed INIT(= FALSE);
 EXTERN HINSTANCE g_hinst INIT(= NULL);
 #endif

src/main.c

@@ -50,9 +50,7 @@ static void exe_pre_commands(mparm_T *parmp);
 static void exe_commands(mparm_T *parmp);
 static void source_startup_scripts(mparm_T *parmp);
 static void main_start_gui(void);
-# if defined(HAS_SWAP_EXISTS_ACTION)
 static void check_swap_exists_action(void);
-# endif
 # ifdef FEAT_EVAL
 static void set_progpath(char_u *argv0);
 # endif
@@ -784,19 +782,15 @@ vim_main2(void)
      */
     if (params.tagname != NULL)
     {
-#if defined(HAS_SWAP_EXISTS_ACTION)
 	swap_exists_did_quit = FALSE;
-#endif
 
 	vim_snprintf((char *)IObuff, IOSIZE, "ta %s", params.tagname);
 	do_cmdline_cmd(IObuff);
 	TIME_MSG("jumping to tag");
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
 	/* If the user doesn't want to edit the file then we quit here. */
 	if (swap_exists_did_quit)
 	    getout(1);
-#endif
     }
 
     /* Execute any "+", "-c" and "-S" arguments. */
@@ -2625,20 +2619,18 @@ read_stdin(void)
 {
     int	    i;
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
+    // When getting the ATTENTION prompt here, use a dialog
-    /* When getting the ATTENTION prompt here, use a dialog */
     swap_exists_action = SEA_DIALOG;
-#endif
+
     no_wait_return = TRUE;
     i = msg_didany;
     set_buflisted(TRUE);
-    (void)open_buffer(TRUE, NULL, 0);	/* create memfile and read file */
+    (void)open_buffer(TRUE, NULL, 0);	// create memfile and read file
     no_wait_return = FALSE;
     msg_didany = i;
     TIME_MSG("reading stdin");
-#if defined(HAS_SWAP_EXISTS_ACTION)
+
     check_swap_exists_action();
-#endif
 #if !(defined(AMIGA) || defined(MACOS_X))
     /*
      * Close stdin and dup it from stderr.  Required for GPM to work
@@ -2741,16 +2733,14 @@ create_windows(mparm_T *parmp UNUSED)
 		if (p_fdls >= 0)
 		    curwin->w_p_fdl = p_fdls;
 #endif
-#if defined(HAS_SWAP_EXISTS_ACTION)
+		// When getting the ATTENTION prompt here, use a dialog
-		/* When getting the ATTENTION prompt here, use a dialog */
 		swap_exists_action = SEA_DIALOG;
-#endif
+
 		set_buflisted(TRUE);
 
 		/* create memfile, read file */
 		(void)open_buffer(FALSE, NULL, 0);
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
 		if (swap_exists_action == SEA_QUIT)
 		{
 		    if (got_int || only_one_window())
@@ -2768,7 +2758,6 @@ create_windows(mparm_T *parmp UNUSED)
 		}
 		else
 		    handle_swap_exists(NULL);
-#endif
 		dorewind = TRUE;		/* start again */
 	    }
 	    ui_breakcheck();
@@ -2865,13 +2854,10 @@ edit_buffers(
 	    curwin->w_arg_idx = arg_idx;
 	    /* Edit file from arg list, if there is one.  When "Quit" selected
 	     * at the ATTENTION prompt close the window. */
-# ifdef HAS_SWAP_EXISTS_ACTION
 	    swap_exists_did_quit = FALSE;
-# endif
 	    (void)do_ecmd(0, arg_idx < GARGCOUNT
 			  ? alist_name(&GARGLIST[arg_idx]) : NULL,
 			  NULL, NULL, ECMD_LASTL, ECMD_HIDE, curwin);
-# ifdef HAS_SWAP_EXISTS_ACTION
 	    if (swap_exists_did_quit)
 	    {
 		/* abort or quit selected */
@@ -2884,7 +2870,6 @@ edit_buffers(
 		win_close(curwin, TRUE);
 		advance = FALSE;
 	    }
-# endif
 	    if (arg_idx == GARGCOUNT - 1)
 		arg_had_last = TRUE;
 	    ++arg_idx;
@@ -3485,7 +3470,6 @@ usage(void)
 	mch_exit(0);
 }
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
 /*
  * Check the result of the ATTENTION dialog:
  * When "Quit" selected, exit Vim.
@@ -3498,7 +3482,6 @@ check_swap_exists_action(void)
 	getout(1);
     handle_swap_exists(NULL);
 }
-#endif
 
 #endif /* NO_VIM_MAIN */
 

src/memline.c

@@ -2159,9 +2159,8 @@ swapfile_info(char_u *fname)
 		{
 		    msg_puts(_("\n        process ID: "));
 		    msg_outnum(char_to_long(b0.b0_pid));
-#if defined(UNIX)
+#if defined(UNIX) || defined(MSWIN)
-		    /* EMX kill() not working correctly, it seems */
+		    if (mch_process_running((pid_t)char_to_long(b0.b0_pid)))
-		    if (kill((pid_t)char_to_long(b0.b0_pid), 0) == 0)
 		    {
 			msg_puts(_(" (STILL RUNNING)"));
 # if defined(FEAT_GUI_DIALOG) || defined(FEAT_CON_DIALOG)
@@ -2193,6 +2192,57 @@ swapfile_info(char_u *fname)
     return x;
 }
 
+/*
+ * Return TRUE if the swap file looks OK and there are no changes, thus it can
+ * be safely deleted.
+ */
+    static time_t
+swapfile_unchanged(char_u *fname)
+{
+    stat_T	    st;
+    int		    fd;
+    struct block0   b0;
+    int		    ret = TRUE;
+#ifdef UNIX
+    long	    pid;
+#endif
+
+    // must be able to stat the swap file
+    if (mch_stat((char *)fname, &st) == -1)
+	return FALSE;
+
+    // must be able to read the first block
+    fd = mch_open((char *)fname, O_RDONLY | O_EXTRA, 0);
+    if (fd < 0)
+	return FALSE;
+    if (read_eintr(fd, &b0, sizeof(b0)) != sizeof(b0))
+    {
+	close(fd);
+	return FALSE;
+    }
+
+    // the ID and magic number must be correct
+    if (ml_check_b0_id(&b0) == FAIL|| b0_magic_wrong(&b0))
+	ret = FALSE;
+
+    // must be unchanged
+    if (b0.b0_dirty)
+	ret = FALSE;
+
+#if defined(UNIX) || defined(MSWIN)
+    // process must known and not be running
+    pid = char_to_long(b0.b0_pid);
+    if (pid == 0L || mch_process_running((pid_t)pid))
+	ret = FALSE;
+#endif
+
+    // TODO: Should we check if the swap file was created on the current
+    // system?  And the current user?
+
+    close(fd);
+    return ret;
+}
+
     static int
 recov_file_names(char_u **names, char_u *path, int prepend_dot)
 {
@@ -4757,9 +4807,8 @@ findswapname(
 		if (differ == FALSE && !(curbuf->b_flags & BF_RECOVERED)
 			&& vim_strchr(p_shm, SHM_ATTENTION) == NULL)
 		{
-#if defined(HAS_SWAP_EXISTS_ACTION)
 		    int		choice = 0;
-#endif
+		    stat_T	st;
 #ifdef CREATE_DUMMY_FILE
 		    int		did_use_dummy = FALSE;
 
@@ -4779,13 +4828,25 @@ findswapname(
 #if (defined(UNIX) || defined(VMS)) && (defined(FEAT_GUI_DIALOG) || defined(FEAT_CON_DIALOG))
 		    process_still_running = FALSE;
 #endif
+		    // It's safe to delete the swap file if all these are true:
+		    // - the edited file exists
+		    // - the swap file has no changes and looks OK
+		    if (mch_stat((char *)buf->b_fname, &st) == 0
+						  && swapfile_unchanged(fname))
+		    {
+			choice = 4;
+			if (p_verbose > 0)
+			    verb_msg(_("Found a swap file that is not useful, deleting it"));
+		    }
+
 #if defined(FEAT_EVAL)
 		    /*
 		     * If there is an SwapExists autocommand and we can handle
 		     * the response, trigger it.  It may return 0 to ask the
 		     * user anyway.
 		     */
-		    if (swap_exists_action != SEA_NONE
+		    if (choice == 0
+			    && swap_exists_action != SEA_NONE
 			    && has_autocmd(EVENT_SWAPEXISTS, buf_fname, buf))
 			choice = do_swapexists(buf, fname);
 
@@ -4850,7 +4911,6 @@ findswapname(
 		    }
 #endif
 
-#if defined(HAS_SWAP_EXISTS_ACTION)
 		    if (choice > 0)
 		    {
 			switch (choice)
@@ -4880,7 +4940,6 @@ findswapname(
 			    break;
 		    }
 		    else
-#endif
 		    {
 			msg_puts("\n");
 			if (msg_silent == 0)

src/os_unix.c

@@ -2393,6 +2393,16 @@ mch_get_pid(void)
     return (long)getpid();
 }
 
+/*
+ * return TRUE if process "pid" is still running
+ */
+    int
+mch_process_running(pid_t pid)
+{
+    // EMX kill() not working correctly, it seems
+    return kill(pid, 0) == 0;
+}
+
 #if !defined(HAVE_STRERROR) && defined(USE_GETCWD)
     static char *
 strerror(int err)

src/os_win32.c

@@ -2903,6 +2903,23 @@ mch_get_pid(void)
     return (long)GetCurrentProcessId();
 }
 
+/*
+ * return TRUE if process "pid" is still running
+ */
+    int
+mch_process_running(pid_t pid)
+{
+    HANDLE  hProcess = OpenProcess(PROCESS_QUERY_INFORMATION, 0, (DWORD)pid);
+    DWORD   status = 0;
+    int	    ret = FALSE;
+
+    if (hProcess == NULL)
+	return FALSE;  // might not have access
+    if (GetExitCodeProcess(hProcess, &status) )
+	ret = status == STILL_ACTIVE;
+    CloseHandle(hProcess);
+    return ret;
+}
 
 /*
  * Get name of current directory into buffer 'buf' of length 'len' bytes.

src/proto/os_unix.pro

@@ -27,6 +27,7 @@ int mch_get_user_name(char_u *s, int len);
 int mch_get_uname(uid_t uid, char_u *s, int len);
 void mch_get_host_name(char_u *s, int len);
 long mch_get_pid(void);
+int mch_process_running(pid_t pid);
 int mch_dirname(char_u *buf, int len);
 int mch_FullName(char_u *fname, char_u *buf, int len, int force);
 int mch_isFullName(char_u *fname);

src/proto/os_win32.pro

@@ -19,6 +19,7 @@ void fname_case(char_u *name, int len);
 int mch_get_user_name(char_u *s, int len);
 void mch_get_host_name(char_u *s, int len);
 long mch_get_pid(void);
+int mch_process_running(pid_t pid);
 int mch_dirname(char_u *buf, int len);
 long mch_getperm(char_u *name);
 int mch_setperm(char_u *name, long perm);

src/testdir/test_swap.vim

@@ -164,3 +164,59 @@ func Test_swapname()
   call delete('Xtest2')
   call delete('Xtest3')
 endfunc
+
+func Test_swapfile_delete()
+  autocmd! SwapExists
+  function s:swap_exists()
+    let v:swapchoice = s:swap_choice
+    let s:swapname = v:swapname
+    let s:filename = expand('<afile>')
+  endfunc
+  augroup test_swapfile_delete
+    autocmd!
+    autocmd SwapExists * call s:swap_exists()
+  augroup END
+
+
+  " Create a valid swapfile by editing a file.
+  split XswapfileText
+  call setline(1, ['one', 'two', 'three'])
+  write  " file is written, not modified
+  " read the swapfile as a Blob
+  let swapfile_name = swapname('%')
+  let swapfile_bytes = readfile(swapfile_name, 'B')
+
+  " Close the file and recreate the swap file.
+  " Now editing the file will run into the process still existing
+  quit
+  call writefile(swapfile_bytes, swapfile_name)
+  let s:swap_choice = 'e'
+  let s:swapname = ''
+  split XswapfileText
+  quit
+  call assert_equal(swapfile_name, s:swapname)
+
+  " Write the swapfile with a modified PID, now it will be automatically
+  " deleted. Process one should never be Vim.
+  let swapfile_bytes[24:27] = 0z01000000
+  call writefile(swapfile_bytes, swapfile_name)
+  let s:swapname = ''
+  split XswapfileText
+  quit
+  call assert_equal('', s:swapname)
+
+  " Now set the modified flag, the swap file will not be deleted
+  let swapfile_bytes[28 + 80 + 899] = 0x55
+  call writefile(swapfile_bytes, swapfile_name)
+  let s:swapname = ''
+  split XswapfileText
+  quit
+  call assert_equal(swapfile_name, s:swapname)
+
+  call delete('XswapfileText')
+  call delete(swapfile_name)
+  augroup test_swapfile_delete
+    autocmd!
+  augroup END
+  augroup! test_swapfile_delete
+endfunc

src/version.c

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