On Fr, 10 Mär 2017, Christian Brabandt wrote:
> On Do, 09 Mär 2017, Christian Brabandt wrote:
>
> > Hi Bram!
> >
> > On Do, 09 Mär 2017, Bram Moolenaar wrote:
> >
> > > Found errors in Test_move_folds_around():
> > > function RunTheTest[21]..Test_move_folds_around line 26: Expected [-1,
> > > -1, -1, -1, -1, -1, -1, 8, 8, 8, 8, 8, -1, 14, 14, 14, 14, 14] but got
> > > [-1, 2, 2, 2, 2, 2, -1, 8, 8, 8, 8, 8, -1, 14, 14, 14, 14, 14]
> > >
> > > Did I miss something? If 7.4.700 should be undone first I suppose the
> > > patch would not apply cleanly, but it did.
> >
> > Sorry, I have probably made a mistake somewhere. I don't have time
> > anymore, so this has to wait until tomorrow.
>
> I have to hold back a bit, apparently the neovim patch isn't complete
> yet.
Updated patch attached.
Best,
Christian
--
Tätig ist man immer mit einen gewissen Lärm. Wirken geht in der Stille
vor sich.
-- Peter Bamm
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
From 1163cf9ef7804b17c78365bcfba30b7fa2deabbe Mon Sep 17 00:00:00 2001
From: Christian Brabandt <[email protected]>
Date: Wed, 8 Mar 2017 21:48:27 +0100
Subject: [PATCH] Fix fold corruption bug when using :move
This is a converted patch from neovim/neovim#6221
Original patch description:
The cause of the underlying bug was that do_move() calls mark_adjust()
to move marks around.
mark_adjust() then calls foldMarkAdjust() to do the same with folds.
foldMarkAdjust() assumes that it is being called for a simple shift
occuring from either deletion or insertion of lines. It hence moves
folds without checking if it causes overlap or leaves folds out of order
in the growarray of folds.
Once folds are out of order, findFold() isn't guaranteed to find the
correct fold, and hence the foldUpdate() call is compromised too.
Similarly, overlapping folds breaks some assumptions in
foldUpdateIEMSRecurse(), but I didn't look into what problems come from
what assumptions.
do_move() is the only place that mark_adjust() is called for something
other than a simple addition or deletion of lines, and hence the only
place that this problem is observed. (do_filter() actually also calls
mark_adjust() in a way that I suspect can cause bugs -- search for "if
(do_in)" and see where marks are moved then deleted -- but I haven't
checked/prooved there's a bug there yet so I haven't fixed it, and the
fix would simply be to swap the order of calls to mark_adjust() around
there)
---
src/ex_cmds.c | 26 +++++--
src/fold.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++
src/mark.c | 26 ++++++-
src/proto/fold.pro | 1 +
src/proto/mark.pro | 1 +
src/testdir/test_fold.vim | 122 +++++++++++++++++++++++++++++++
6 files changed, 350 insertions(+), 7 deletions(-)
diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index 564a79efc..df35f52da 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -798,6 +798,10 @@ do_move(linenr_T line1, linenr_T line2, linenr_T dest)
linenr_T extra; /* Num lines added before line1 */
linenr_T num_lines; /* Num lines moved */
linenr_T last_line; /* Last line in file after adding new text */
+#ifdef FEAT_FOLDING
+ win_T *win;
+ tabpage_T *tp;
+#endif
if (dest >= line1 && dest < line2)
{
@@ -841,24 +845,34 @@ do_move(linenr_T line1, linenr_T line2, linenr_T dest)
* their final destination at the new text position -- webb
*/
last_line = curbuf->b_ml.ml_line_count;
- mark_adjust(line1, line2, last_line - line2, 0L);
- changed_lines(last_line - num_lines + 1, 0, last_line + 1, num_lines);
+ mark_adjust_nofold(line1, line2, last_line - line2, 0L);
if (dest >= line2)
{
- mark_adjust(line2 + 1, dest, -num_lines, 0L);
+ mark_adjust_nofold(line2 + 1, dest, -num_lines, 0L);
+#ifdef FEAT_FOLDING
+ FOR_ALL_TAB_WINDOWS(tp, win) {
+ if (win->w_buffer == curbuf)
+ foldMoveRange(&win->w_folds, line1, line2, dest);
+ }
+#endif
curbuf->b_op_start.lnum = dest - num_lines + 1;
curbuf->b_op_end.lnum = dest;
}
else
{
- mark_adjust(dest + 1, line1 - 1, num_lines, 0L);
+ mark_adjust_nofold(dest + 1, line1 - 1, num_lines, 0L);
+#ifdef FEAT_FOLDING
+ FOR_ALL_TAB_WINDOWS(tp, win) {
+ if (win->w_buffer == curbuf)
+ foldMoveRange(&win->w_folds, dest + 1, line1 - 1, line2);
+ }
+#endif
curbuf->b_op_start.lnum = dest + 1;
curbuf->b_op_end.lnum = dest + num_lines;
}
curbuf->b_op_start.col = curbuf->b_op_end.col = 0;
- mark_adjust(last_line - num_lines + 1, last_line,
+ mark_adjust_nofold(last_line - num_lines + 1, last_line,
-(last_line - dest - extra), 0L);
- changed_lines(last_line - num_lines + 1, 0, last_line + 1, -extra);
/*
* Now we delete the original text -- webb
diff --git a/src/fold.c b/src/fold.c
index 826ad5c8d..1ef0ef0c3 100644
--- a/src/fold.c
+++ b/src/fold.c
@@ -64,6 +64,7 @@ static void deleteFoldMarkers(fold_T *fp, int recursive, linenr_T lnum_off);
static void foldDelMarker(linenr_T lnum, char_u *marker, int markerlen);
static void foldUpdateIEMS(win_T *wp, linenr_T top, linenr_T bot);
static void parseMarker(win_T *wp);
+static void foldMoveRange_int(garray_T *gap, linenr_T line1, linenr_T line2, linenr_T dest);
static char *e_nofold = N_("E490: No fold found");
@@ -1075,6 +1076,12 @@ foldAdjustCursor(void)
(void)hasFolding(curwin->w_cursor.lnum, &curwin->w_cursor.lnum, NULL);
}
+/* foldMoveRange() {{{2 */
+ void
+foldMoveRange(garray_T *gap, linenr_T line1, linenr_T line2, linenr_T dest)
+{
+ foldMoveRange_int(gap, line1, line2, dest);
+}
/* Internal functions for "fold_T" {{{1 */
/* cloneFoldGrowArray() {{{2 */
/*
@@ -2968,6 +2975,180 @@ foldRemove(garray_T *gap, linenr_T top, linenr_T bot)
}
}
+/* foldReverseOrder() {{{2 */
+ static void
+foldReverseOrder(garray_T *gap, linenr_T start, linenr_T end)
+{
+ fold_T *left, *right;
+ fold_T tmp;
+
+ for (; start < end; start++, end--) {
+ left = (fold_T *)gap->ga_data + start;
+ right = (fold_T *)gap->ga_data + end;
+ tmp = *left;
+ *left = *right;
+ *right = tmp;
+ }
+}
+
+/* foldMoveRange_int() {{{2 */
+/*
+ * Move folds within the inclusive range "line1" to "line2" to after "dest"
+ * requires "line1" <= "line2" <= "dest"
+ *
+ * There are the following situations for the first fold at or below line1 - 1.
+ * 1 2 3 4
+ * 1 2 3 4
+ * line1 2 3 4
+ * 2 3 4 5 6 7
+ * line2 3 4 5 6 7
+ * 3 4 6 7 8 9
+ * dest 4 7 8 9
+ * 4 7 8 10
+ * 4 7 8 10
+ *
+ * In the following descriptions, "moved" means moving in the buffer, *and* in
+ * the fold array.
+ * Meanwhile, "shifted" just means moving in the buffer.
+ * 1. not changed
+ * 2. truncated above line1
+ * 3. length reduced by line2 - line1, folds starting between the end of 3 and
+ * dest are truncated and shifted up
+ * 4. internal folds moved (from [line1, line2] to dest)
+ * 5. moved to dest.
+ * 6. truncated below line2 and moved.
+ * 7. length reduced by line2 - dest, folds starting between line2 and dest are
+ * removed, top is moved down by move_len.
+ * 8. truncated below dest and shifted up.
+ * 9. shifted up
+ * 10. not changed
+ */
+
+ static void
+truncate_fold(fold_T *fp, linenr_T end)
+{
+ foldRemove(&fp->fd_nested, end - fp->fd_top, MAXLNUM);
+ fp->fd_len = end - fp->fd_top + 1;
+}
+
+#define fold_end(fp) ((fp)->fd_top + (fp)->fd_len - 1)
+#define valid_fold(fp, gap) ((fp) < ((fold_T *)(gap)->ga_data + (gap)->ga_len))
+#define fold_index(fp, gap) ((size_t)(fp - ((fold_T *)(gap)->ga_data)))
+
+ static void
+foldMoveRange_int(garray_T *gap, linenr_T line1, linenr_T line2, linenr_T dest)
+{
+ fold_T *fp;
+ linenr_T range_len = line2 - line1 + 1;
+ linenr_T move_len = dest - line2;
+ int at_start = foldFind(gap, line1 - 1, &fp);
+ size_t move_start = 0, move_end = 0, dest_index = 0;
+
+ if (at_start) {
+ if (fold_end(fp) > dest)
+ {
+ /* Case 4
+ * don't have to change this fold, but have to move nested folds.
+ */
+ foldMoveRange(&fp->fd_nested, line1 - fp->fd_top, line2 -
+ fp->fd_top, dest - fp->fd_top);
+ return;
+ }
+ else if (fold_end(fp) > line2)
+ {
+ /* Case 3
+ * Remove nested folds between line1 and line2 & reduce the
+ * length of fold by "range_len".
+ * Folds after this one must be dealt with.
+ */
+ foldMarkAdjustRecurse(&fp->fd_nested, line1 - fp->fd_top, line2 -
+ fp->fd_top, MAXLNUM, -range_len);
+ fp->fd_len -= range_len;
+ }
+ else
+ /* Case 2 truncate fold, folds after this one must be dealt with. */
+ truncate_fold(fp, line1);
+
+ /* Look at the next fold, and treat that one as if it were the first
+ * after "line1" (because now it is). */
+ fp = fp + 1;
+ }
+
+ if (!valid_fold(fp, gap) || fp->fd_top > dest)
+ {
+ /* Case 10
+ * No folds after "line1" and before "dest"
+ */
+ return;
+ }
+ else if (fp->fd_top > line2)
+ {
+ for (; valid_fold(fp, gap) && fold_end(fp) < dest; fp++)
+ /* Case 9. (for all case 9's) -- shift up. */
+ fp->fd_top -= range_len;
+
+ if (valid_fold(fp, gap) && fp->fd_top < dest)
+ {
+ /* Case 8. -- ensure truncated at dest, shift up */
+ truncate_fold(fp, dest);
+ fp->fd_top -= range_len;
+ }
+ return;
+ }
+ else if (fold_end(fp) > dest)
+ {
+ /* Case 7 -- remove nested folds and shrink */
+ foldMarkAdjustRecurse(&fp->fd_nested, line2 + 1 - fp->fd_top, dest -
+ fp->fd_top, MAXLNUM, -move_len);
+ fp->fd_len -= move_len;
+ fp->fd_top += move_len;
+ return;
+ }
+
+ /* Case 5 or 6
+ * changes rely on whether there are folds between the end of
+ * this fold and "dest".
+ */
+ move_start = fold_index(fp, gap);
+
+ for (; valid_fold(fp, gap) && fp->fd_top <= dest; fp++)
+ {
+ if (fp->fd_top <= line2)
+ {
+ /* 1. 2. or 3. */
+ if (fold_end(fp) > line2)
+ /* 2. or 3., truncate before moving */
+ truncate_fold(fp, line2);
+
+ fp->fd_top += move_len;
+ continue;
+ }
+
+ /* Record index of the first fold after the moved range. */
+ if (move_end == 0)
+ move_end = fold_index(fp, gap);
+
+ if (fold_end(fp) > dest)
+ truncate_fold(fp, dest);
+
+ fp->fd_top -= range_len;
+ }
+
+ dest_index = fold_index(fp, gap);
+
+ /*
+ * All folds are now correct, but they are not necessarily in the correct
+ * order. We have to swap folds in the range [move_end, dest_index) with
+ * those in the range [move_start, move_end).
+ */
+ foldReverseOrder(gap, move_start, dest_index - 1);
+ foldReverseOrder(gap, move_start, move_start + dest_index - move_end - 1);
+ foldReverseOrder(gap, move_start + dest_index - move_end, dest_index - 1);
+}
+#undef fold_end
+#undef valid_fold
+#undef fold_index
+
/* foldMerge() {{{2 */
/*
* Merge two adjacent folds (and the nested ones in them).
diff --git a/src/mark.c b/src/mark.c
index 194125eb0..0d21305bc 100644
--- a/src/mark.c
+++ b/src/mark.c
@@ -37,6 +37,8 @@ static void cleanup_jumplist(void);
#ifdef FEAT_VIMINFO
static void write_one_filemark(FILE *fp, xfmark_T *fm, int c1, int c2);
#endif
+static void mark_adjust_internal(linenr_T line1, linenr_T line2, long amount,
+ long amount_after, int adjust_folds);
/*
* Set named mark "c" at current cursor position.
@@ -1029,6 +1031,27 @@ mark_adjust(
long amount,
long amount_after)
{
+ mark_adjust_internal(line1, line2, amount, amount_after, TRUE);
+}
+
+ void
+mark_adjust_nofold(
+ linenr_T line1,
+ linenr_T line2,
+ long amount,
+ long amount_after)
+{
+ mark_adjust_internal(line1, line2, amount, amount_after, FALSE);
+}
+
+ static void
+mark_adjust_internal(
+ linenr_T line1,
+ linenr_T line2,
+ long amount,
+ long amount_after,
+ int adjust_folds UNUSED)
+{
int i;
int fnum = curbuf->b_fnum;
linenr_T *lp;
@@ -1174,7 +1197,8 @@ mark_adjust(
#ifdef FEAT_FOLDING
/* adjust folds */
- foldMarkAdjust(win, line1, line2, amount, amount_after);
+ if (adjust_folds)
+ foldMarkAdjust(win, line1, line2, amount, amount_after);
#endif
}
}
diff --git a/src/proto/fold.pro b/src/proto/fold.pro
index dce180381..cda536b27 100644
--- a/src/proto/fold.pro
+++ b/src/proto/fold.pro
@@ -31,6 +31,7 @@ void foldInitWin(win_T *new_win);
int find_wl_entry(win_T *win, linenr_T lnum);
void foldAdjustVisual(void);
void foldAdjustCursor(void);
+void foldMoveRange(garray_T *gap, linenr_T line1, linenr_T line2, linenr_T dest);
void cloneFoldGrowArray(garray_T *from, garray_T *to);
void deleteFoldRecurse(garray_T *gap);
void foldMarkAdjust(win_T *wp, linenr_T line1, linenr_T line2, long amount, long amount_after);
diff --git a/src/proto/mark.pro b/src/proto/mark.pro
index 90be1a566..ff1f441e8 100644
--- a/src/proto/mark.pro
+++ b/src/proto/mark.pro
@@ -19,6 +19,7 @@ void ex_jumps(exarg_T *eap);
void ex_clearjumps(exarg_T *eap);
void ex_changes(exarg_T *eap);
void mark_adjust(linenr_T line1, linenr_T line2, long amount, long amount_after);
+void mark_adjust_nofold(linenr_T line1, linenr_T line2, long amount, long amount_after);
void mark_col_adjust(linenr_T lnum, colnr_T mincol, long lnum_amount, long col_amount);
void copy_jumplist(win_T *from, win_T *to);
void free_jumplist(win_T *wp);
diff --git a/src/testdir/test_fold.vim b/src/testdir/test_fold.vim
index f10480bb4..19c94140d 100644
--- a/src/testdir/test_fold.vim
+++ b/src/testdir/test_fold.vim
@@ -1,5 +1,9 @@
" Test for folding
+func! PrepIndent(arg)
+ return [a:arg] + repeat(["\t".a:arg], 5)
+endfu
+
func! Test_address_fold()
new
call setline(1, ['int FuncName() {/*{{{*/', 1, 2, 3, 4, 5, '}/*}}}*/',
@@ -219,3 +223,121 @@ func Test_update_folds_expr_read()
bwipe!
set foldmethod& foldexpr&
endfunc
+
+func! Test_move_folds_around_manual()
+ new
+ let input = PrepIndent("a") + PrepIndent("b") + PrepIndent("c")
+ call setline(1, PrepIndent("a") + PrepIndent("b") + PrepIndent("c"))
+ let folds=[-1, 2, 2, 2, 2, 2, -1, 8, 8, 8, 8, 8, -1, 14, 14, 14, 14, 14]
+ " all folds closed
+ set foldenable foldlevel=0 fdm=indent
+ " needs a forced redraw
+ redraw!
+ set fdm=manual
+ call assert_equal(folds, map(range(1, line('$')), 'foldclosed(v:val)'))
+ call assert_equal(input, getline(1, '$'))
+ 7,12m0
+ call assert_equal(PrepIndent("b") + PrepIndent("a") + PrepIndent("c"), getline(1, '$'))
+ call assert_equal(folds, map(range(1, line('$')), 'foldclosed(v:val)'))
+ 10,12m0
+ call assert_equal(PrepIndent("a")[1:] + PrepIndent("b") + ["a"] + PrepIndent("c"), getline(1, '$'))
+ call assert_equal([1, 1, 1, 1, 1, -1, 7, 7, 7, 7, 7, -1, -1, 14, 14, 14, 14, 14], map(range(1, line('$')), 'foldclosed(v:val)'))
+ " moving should not close the folds
+ %d
+ call setline(1, PrepIndent("a") + PrepIndent("b") + PrepIndent("c"))
+ set fdm=indent
+ redraw!
+ set fdm=manual
+ call cursor(2, 1)
+ norm! zR
+ 7,12m0
+ let folds=repeat([-1], 18)
+ call assert_equal(PrepIndent("b") + PrepIndent("a") + PrepIndent("c"), getline(1, '$'))
+ call assert_equal(folds, map(range(1, line('$')), 'foldclosed(v:val)'))
+ norm! zM
+ " folds are not corrupted and all have been closed
+ call assert_equal([-1, 2, 2, 2, 2, 2, -1, 8, 8, 8, 8, 8, -1, 14, 14, 14, 14, 14], map(range(1, line('$')), 'foldclosed(v:val)'))
+ %d
+ call setline(1, ["a", "\tb", "\tc", "\td", "\te"])
+ set fdm=indent
+ redraw!
+ set fdm=manual
+ %foldopen
+ 3m4
+ %foldclose
+ call assert_equal(["a", "\tb", "\td", "\tc", "\te"], getline(1, '$'))
+ call assert_equal([-1, 5, 5, 5, 5], map(range(1, line('$')), 'foldclosedend(v:val)'))
+ %d
+ call setline(1, ["a", "\tb", "\tc", "\td", "\te", "z", "\ty", "\tx", "\tw", "\tv"])
+ set fdm=indent foldlevel=0
+ set fdm=manual
+ %foldopen
+ 3m1
+ %foldclose
+ call assert_equal(["a", "\tc", "\tb", "\td", "\te", "z", "\ty", "\tx", "\tw", "\tv"], getline(1, '$'))
+ call assert_equal(0, foldlevel(2))
+ call assert_equal(5, foldclosedend(3))
+ call assert_equal([-1, -1, 3, 3, 3, -1, 7, 7, 7, 7], map(range(1, line('$')), 'foldclosed(v:val)'))
+ 2,6m$
+ %foldclose
+ call assert_equal(5, foldclosedend(2))
+ call assert_equal(0, foldlevel(6))
+ call assert_equal(9, foldclosedend(7))
+ call assert_equal([-1, 2, 2, 2, 2, -1, 7, 7, 7, -1], map(range(1, line('$')), 'foldclosed(v:val)'))
+ bw!
+endfunc
+
+func! Test_move_folds_around_indent()
+ new
+ let input = PrepIndent("a") + PrepIndent("b") + PrepIndent("c")
+ call setline(1, PrepIndent("a") + PrepIndent("b") + PrepIndent("c"))
+ let folds=[-1, 2, 2, 2, 2, 2, -1, 8, 8, 8, 8, 8, -1, 14, 14, 14, 14, 14]
+ " all folds closed
+ set fdm=indent
+ call assert_equal(folds, map(range(1, line('$')), 'foldclosed(v:val)'))
+ call assert_equal(input, getline(1, '$'))
+ 7,12m0
+ call assert_equal(PrepIndent("b") + PrepIndent("a") + PrepIndent("c"), getline(1, '$'))
+ call assert_equal(folds, map(range(1, line('$')), 'foldclosed(v:val)'))
+ 10,12m0
+ call assert_equal(PrepIndent("a")[1:] + PrepIndent("b") + ["a"] + PrepIndent("c"), getline(1, '$'))
+ call assert_equal([1, 1, 1, 1, 1, -1, 7, 7, 7, 7, 7, -1, -1, 14, 14, 14, 14, 14], map(range(1, line('$')), 'foldclosed(v:val)'))
+ " moving should not close the folds
+ %d
+ call setline(1, PrepIndent("a") + PrepIndent("b") + PrepIndent("c"))
+ set fdm=indent
+ call cursor(2, 1)
+ norm! zR
+ 7,12m0
+ let folds=repeat([-1], 18)
+ call assert_equal(PrepIndent("b") + PrepIndent("a") + PrepIndent("c"), getline(1, '$'))
+ call assert_equal(folds, map(range(1, line('$')), 'foldclosed(v:val)'))
+ norm! zM
+ " folds are not corrupted and all have been closed
+ call assert_equal([-1, 2, 2, 2, 2, 2, -1, 8, 8, 8, 8, 8, -1, 14, 14, 14, 14, 14], map(range(1, line('$')), 'foldclosed(v:val)'))
+ %d
+ call setline(1, ["a", "\tb", "\tc", "\td", "\te"])
+ set fdm=indent
+ %foldopen
+ 3m4
+ %foldclose
+ call assert_equal(["a", "\tb", "\td", "\tc", "\te"], getline(1, '$'))
+ call assert_equal([-1, 5, 5, 5, 5], map(range(1, line('$')), 'foldclosedend(v:val)'))
+ %d
+ call setline(1, ["a", "\tb", "\tc", "\td", "\te", "z", "\ty", "\tx", "\tw", "\tv"])
+ set fdm=indent foldlevel=0
+ %foldopen
+ 3m1
+ %foldclose
+ call assert_equal(["a", "\tc", "\tb", "\td", "\te", "z", "\ty", "\tx", "\tw", "\tv"], getline(1, '$'))
+ call assert_equal(1, foldlevel(2))
+ call assert_equal(5, foldclosedend(3))
+ call assert_equal([-1, 2, 2, 2, 2, -1, 7, 7, 7, 7], map(range(1, line('$')), 'foldclosed(v:val)'))
+ 2,6m$
+ %foldclose
+ call assert_equal(9, foldclosedend(2))
+ call assert_equal(1, foldlevel(6))
+ call assert_equal(9, foldclosedend(7))
+ call assert_equal([-1, 2, 2, 2, 2, 2, 2, 2, 2, -1], map(range(1, line('$')), 'foldclosed(v:val)'))
+ bw!
+endfunc
--
2.11.0
