From 8b8ecf44f2cda43bbd710ec22ef99439b71888cd Mon Sep 17 00:00:00 2001
From: Abdelhakeem Osama <abdelhakeem.osama@hotmail.com>
Date: Tue, 3 Sep 2019 19:18:24 +0200
Subject: shada/context: fully remove jumplist duplicates #10898

- Always load files when cleaning up jumplist.
  - For Shada: avoids writing duplicate entries, which happens when you read
    from a shada file with duplicate entries (merging the jumplist while
    writing sometimes produces duplicate entries, bug?) and then write right
    away (i.e.: without any `:jumps`, `getjumplist()`, or any jump movement,
    that is: nothing that calls `cleanup_jumplist` with `loadfiles == true`).
  - For Context: avoids non-idempotent behavior for the same reason (i.e.:
    first call to `shada_encode_jumps` does not remove duplicate entries).

- Do not set pcmark when dumping jumplist for Context.
  - Retrieving current Context shouldn't add an entry to the jumplist
    (which will be removed by a subsequent `cleanup_jumplist` anyway, i.e.:
    tail entry matching current position), just act like `getjumplist` for
    instance.
---
 src/nvim/mark.c  | 25 +++++++++++--------------
 src/nvim/shada.c | 11 ++++++++---
 2 files changed, 19 insertions(+), 17 deletions(-)

(limited to 'src')

diff --git a/src/nvim/mark.c b/src/nvim/mark.c
index 9f357575d0..e103d3cb55 100644
--- a/src/nvim/mark.c
+++ b/src/nvim/mark.c
@@ -1162,21 +1162,18 @@ void mark_col_adjust(
 
 // When deleting lines, this may create duplicate marks in the
 // jumplist. They will be removed here for the specified window.
-// When "loadfiles" is true first ensure entries have the "fnum" field set
-// (this may be a bit slow).
-void cleanup_jumplist(win_T *wp, bool loadfiles)
+// When "checktail" is true, removes tail jump if it matches current position.
+void cleanup_jumplist(win_T *wp, bool checktail)
 {
   int i;
 
-  if (loadfiles) {
-    // If specified, load all the files from the jump list. This is
-    // needed to properly clean up duplicate entries, but will take some
-    // time.
-    for (i = 0; i < wp->w_jumplistlen; i++) {
-      if ((wp->w_jumplist[i].fmark.fnum == 0)
-          && (wp->w_jumplist[i].fmark.mark.lnum != 0)) {
-        fname2fnum(&wp->w_jumplist[i]);
-      }
+  // Load all the files from the jump list. This is
+  // needed to properly clean up duplicate entries, but will take some
+  // time.
+  for (i = 0; i < wp->w_jumplistlen; i++) {
+    if ((wp->w_jumplist[i].fmark.fnum == 0)
+        && (wp->w_jumplist[i].fmark.mark.lnum != 0)) {
+      fname2fnum(&wp->w_jumplist[i]);
     }
   }
 
@@ -1213,8 +1210,8 @@ void cleanup_jumplist(win_T *wp, bool loadfiles)
 
   // When pointer is below last jump, remove the jump if it matches the current
   // line.  This avoids useless/phantom jumps. #9805
-  if (loadfiles  // otherwise (i.e.: Shada), last entry should be kept
-      && wp->w_jumplistlen && wp->w_jumplistidx == wp->w_jumplistlen) {
+  if (checktail && wp->w_jumplistlen
+      && wp->w_jumplistidx == wp->w_jumplistlen) {
     const xfmark_T *fm_last = &wp->w_jumplist[wp->w_jumplistlen - 1];
     if (fm_last->fmark.fnum == curbuf->b_fnum
         && fm_last->fmark.mark.lnum == wp->w_cursor.lnum) {
diff --git a/src/nvim/shada.c b/src/nvim/shada.c
index 50f8990cf6..7e7e7cfdf7 100644
--- a/src/nvim/shada.c
+++ b/src/nvim/shada.c
@@ -2737,6 +2737,8 @@ static ShaDaWriteResult shada_write(ShaDaWriteDef *const sd_writer,
   }
 
   // Initialize jump list
+  setpcmark();
+  cleanup_jumplist(curwin, false);
   wms->jumps_size = shada_init_jumps(wms->jumps, &removable_bufs);
 
   // Initialize global marks
@@ -4086,11 +4088,13 @@ static bool shada_removable(const char *name)
 static inline size_t shada_init_jumps(
     PossiblyFreedShadaEntry *jumps, khash_t(bufset) *const removable_bufs)
 {
-  // Initialize jump list
+  if (!curwin->w_jumplistlen) {
+    return 0;
+  }
+
   size_t jumps_size = 0;
   const void *jump_iter = NULL;
-  setpcmark();
-  cleanup_jumplist(curwin, false);
+
   do {
     xfmark_T fm;
     jump_iter = mark_jumplist_iter(jump_iter, curwin, &fm);
@@ -4164,6 +4168,7 @@ void shada_encode_jumps(msgpack_sbuffer *const sbuf)
   khash_t(bufset) removable_bufs = KHASH_EMPTY_TABLE(bufset);
   find_removable_bufs(&removable_bufs);
   PossiblyFreedShadaEntry jumps[JUMPLISTSIZE];
+  cleanup_jumplist(curwin, true);
   size_t jumps_size = shada_init_jumps(jumps, &removable_bufs);
   msgpack_packer packer;
   msgpack_packer_init(&packer, sbuf, msgpack_sbuffer_write);
-- 
cgit