patch 8.1.2126: viminfo not sufficiently tested

Problem:    Viminfo not sufficiently tested.
Solution:   Add more test cases.  Clean up comments. (Yegappan Lakshmanan,
            closes #5032)

src/search.c

@@ -5783,12 +5783,18 @@ show_pat_in_path(
 #endif
 
 #ifdef FEAT_VIMINFO
+/*
+ * Return the last used search pattern at "idx".
+ */
     spat_T *
 get_spat(int idx)
 {
     return &spats[idx];
 }
 
+/*
+ * Return the last used search pattern index.
+ */
     int
 get_spat_last_idx(void)
 {

src/structs.h

@@ -1133,18 +1133,6 @@ typedef struct
     int		vc_fail;	// fail for invalid char, don't use '?'
 } vimconv_T;
 
-/*
- * Structure used for reading from the viminfo file.
- */
-typedef struct
-{
-    char_u	*vir_line;	// text of the current line
-    FILE	*vir_fd;	// file descriptor
-    vimconv_T	vir_conv;	// encoding conversion
-    int		vir_version;	// viminfo version detected or -1
-    garray_T	vir_barlines;	// lines starting with |
-} vir_T;
-
 /*
  * Structure used for the command line history.
  */

src/testdir/test_viminfo.vim

@@ -1,5 +1,7 @@
 " Test for reading and writing .viminfo
 
+source check.vim
+
 function Test_viminfo_read_and_write()
   " First clear 'history', so that "hislen" is zero.  Then set it again,
   " simulating Vim starting up.
@@ -715,26 +717,78 @@ func Test_viminfo_large_register()
   rviminfo! Xviminfo
   call assert_equal(join(repeat(["sun is rising"], 200), "\n"), @r)
   call delete('Xviminfo')
+  let @r = ''
   let &viminfo = save_viminfo
 endfunc
 
 " Test for setting 'viminfofile' to NONE
 func Test_viminfofile_none()
+  let save_vif = &viminfofile
   set viminfofile=NONE
   wviminfo Xviminfo
   call assert_false(filereadable('Xviminfo'))
   call writefile([''], 'Xviminfo')
   call assert_fails('rviminfo Xviminfo', 'E195:')
   call delete('Xviminfo')
+  let &viminfofile = save_vif
 endfunc
 
-" Test for an unwritable 'viminfo' file
+" Test for an unwritable and unreadble 'viminfo' file
-func Test_viminfo_readonly()
+func Test_viminfo_perm()
-  if !has('unix')
+  CheckUnix
-      return
-  endif
   call writefile([''], 'Xviminfo')
   call setfperm('Xviminfo', 'r-x------')
   call assert_fails('wviminfo Xviminfo', 'E137:')
+  call setfperm('Xviminfo', '--x------')
+  call assert_fails('rviminfo Xviminfo', 'E195:')
   call delete('Xviminfo')
 endfunc
+
+" Test for writing to an existing viminfo file merges the file marks
+func XTest_viminfo_marks_merge()
+  let save_viminfo = &viminfo
+  set viminfo&vim
+  set viminfo^=%
+  enew
+  %argdelete
+  %bwipe
+
+  call writefile(repeat(['editor'], 10), 'Xbufa')
+  call writefile(repeat(['Vim'], 10), 'Xbufb')
+
+  " set marks in buffers
+  call test_settime(10)
+  edit Xbufa
+  4mark a
+  wviminfo Xviminfo
+  edit Xbufb
+  4mark b
+  wviminfo Xviminfo
+  %bwipe
+
+  " set marks in buffers again
+  call test_settime(20)
+  edit Xbufb
+  6mark b
+  wviminfo Xviminfo
+  edit Xbufa
+  6mark a
+  wviminfo Xviminfo
+  %bwipe
+
+  " Load the buffer and check the marks
+  edit Xbufa
+  rviminfo! Xviminfo
+  call assert_equal(6, line("'a"))
+  edit Xbufb
+  rviminfo! Xviminfo
+  call assert_equal(6, line("'b"))
+
+  " cleanup
+  %bwipe
+  call delete('Xviminfo')
+  call delete('Xbufa')
+  call delete('Xbufb')
+  call test_settime(0)
+  let &viminfo=save_viminfo
+endfunc

src/version.c

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

src/viminfo.c

@@ -14,6 +14,18 @@
 #include "vim.h"
 #include "version.h"
 
+/*
+ * Structure used for reading from the viminfo file.
+ */
+typedef struct
+{
+    char_u	*vir_line;	// text of the current line
+    FILE	*vir_fd;	// file descriptor
+    vimconv_T	vir_conv;	// encoding conversion
+    int		vir_version;	// viminfo version detected or -1
+    garray_T	vir_barlines;	// lines starting with |
+} vir_T;
+
 #if defined(FEAT_VIMINFO) || defined(PROTO)
 
 static int  viminfo_errcnt;
@@ -822,11 +834,9 @@ write_viminfo_history(FILE *fp, int merge)
 	if (num_saved > hislen)
 	    num_saved = hislen;
 
-	/*
+	// Merge typed and viminfo history:
-	 * Merge typed and viminfo history:
+	// round 1: history of typed commands.
-	 * round 1: history of typed commands.
+	// round 2: history from recently read viminfo.
-	 * round 2: history from recently read viminfo.
-	 */
 	for (round = 1; round <= 2; ++round)
 	{
 	    if (round == 1)
@@ -2671,16 +2681,14 @@ read_viminfo_barline(vir_T *virp, int got_encoding, int force, int writing)
     int		i;
     int		read_next = TRUE;
 
-    /*
+    // The format is: |{bartype},{value},...
-     * The format is: |{bartype},{value},...
+    // For a very long string:
-     * For a very long string:
+    //     |{bartype},>{length of "{text}{text2}"}
-     *     |{bartype},>{length of "{text}{text2}"}
+    //     |<{text1}
-     *     |<{text1}
+    //     |<{text2},{value}
-     *     |<{text2},{value}
+    // For a long line not using a string
-     * For a long line not using a string
+    //     |{bartype},{lots of values},>
-     *     |{bartype},{lots of values},>
+    //     |<{value},{value}
-     *     |<{value},{value}
-     */
     if (*p == '<')
     {
 	// Continuation line of an unrecognized item.
@@ -3032,18 +3040,14 @@ write_viminfo(char_u *file, int forceit)
     }
     else
     {
-	/*
+	// There is an existing viminfo file.  Create a temporary file to
-	 * There is an existing viminfo file.  Create a temporary file to
+	// write the new viminfo into, in the same directory as the
-	 * write the new viminfo into, in the same directory as the
+	// existing viminfo file, which will be renamed once all writing is
-	 * existing viminfo file, which will be renamed once all writing is
+	// successful.
-	 * successful.
-	 */
 #ifdef UNIX
-	/*
+	// For Unix we check the owner of the file.  It's not very nice to
-	 * For Unix we check the owner of the file.  It's not very nice to
+	// overwrite a user's viminfo file after a "su root", with a
-	 * overwrite a user's viminfo file after a "su root", with a
+	// viminfo file that the user can't read.
-	 * viminfo file that the user can't read.
-	 */
 	st_old.st_dev = (dev_t)0;
 	st_old.st_ino = 0;
 	st_old.st_mode = 0600;
@@ -3069,14 +3073,12 @@ write_viminfo(char_u *file, int forceit)
 	hidden = mch_ishidden(fname);
 #endif
 
-	/*
+	// Make tempname, find one that does not exist yet.
-	 * Make tempname, find one that does not exist yet.
+	// Beware of a race condition: If someone logs out and all Vim
-	 * Beware of a race condition: If someone logs out and all Vim
+	// instances exit at the same time a temp file might be created between
-	 * instances exit at the same time a temp file might be created between
+	// stat() and open().  Use mch_open() with O_EXCL to avoid that.
-	 * stat() and open().  Use mch_open() with O_EXCL to avoid that.
+	// May try twice: Once normal and once with shortname set, just in
-	 * May try twice: Once normal and once with shortname set, just in
+	// case somebody puts his viminfo file in an 8.3 filesystem.
-	 * case somebody puts his viminfo file in an 8.3 filesystem.
-	 */
 	for (;;)
 	{
 	    int		next_char = 'z';
@@ -3098,30 +3100,24 @@ write_viminfo(char_u *file, int forceit)
 	    if (tempname == NULL)		// out of memory
 		break;
 
-	    /*
+	    // Try a series of names.  Change one character, just before
-	     * Try a series of names.  Change one character, just before
+	    // the extension.  This should also work for an 8.3
-	     * the extension.  This should also work for an 8.3
+	    // file name, when after adding the extension it still is
-	     * file name, when after adding the extension it still is
+	    // the same file as the original.
-	     * the same file as the original.
-	     */
 	    wp = tempname + STRLEN(tempname) - 5;
 	    if (wp < gettail(tempname))	    // empty file name?
 		wp = gettail(tempname);
 	    for (;;)
 	    {
-		/*
+		// Check if tempfile already exists.  Never overwrite an
-		 * Check if tempfile already exists.  Never overwrite an
+		// existing file!
-		 * existing file!
-		 */
 		if (mch_stat((char *)tempname, &st_new) == 0)
 		{
 #ifdef UNIX
-		    /*
+		    // Check if tempfile is same as original file.  May happen
-		     * Check if tempfile is same as original file.  May happen
+		    // when modname() gave the same file back.  E.g.  silly
-		     * when modname() gave the same file back.  E.g.  silly
+		    // link, or file name-length reached.  Try again with
-		     * link, or file name-length reached.  Try again with
+		    // shortname set.
-		     * shortname set.
-		     */
 		    if (!shortname && st_new.st_dev == st_old.st_dev
 						&& st_new.st_ino == st_old.st_ino)
 		    {
@@ -3199,10 +3195,8 @@ write_viminfo(char_u *file, int forceit)
 	{
 		stat_T	tmp_st;
 
-	    /*
+	    // Make sure the original owner can read/write the tempfile and
-	     * Make sure the original owner can read/write the tempfile and
+	    // otherwise preserve permissions, making sure the group matches.
-	     * otherwise preserve permissions, making sure the group matches.
-	     */
 	    if (mch_stat((char *)tempname, &tmp_st) >= 0)
 	    {
 		if (st_old.st_uid != tmp_st.st_uid)
@@ -3222,9 +3216,7 @@ write_viminfo(char_u *file, int forceit)
 #endif
     }
 
-    /*
+    // Check if the new viminfo file can be written to.
-     * Check if the new viminfo file can be written to.
-     */
     if (fp_out == NULL)
     {
 	semsg(_("E138: Can't write viminfo file %s!"),