On Mi, 08 Mär 2017, Christian Brabandt wrote: > On Mo, 06 Mär 2017, Bram Moolenaar wrote: > > > > > Christian Brabandt wrote: > > > > > And another one that fixes the :move closes folds and :move invalidates > > > folds: > > > https://github.com/neovim/neovim/pull/6221 > > > > It's a bit big, but I suppose it's needed to make it work. > > > > Can someone turn this into a Vim patch? And check that it works, > > removing the changed_lines() calls is a bit unexpected.
That is called later down anyways, so I guess it is okay. > I'll check it out within the next days. Converted patch attached. I slightly renamed some functions, added some static declarations and added some ifdefs and wrote a new test. From my testing it looks good now. Best, Christian -- Wer übertriebenen Wert auf Pünktlichkeit legt, ist viel allein. -- -- 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 b712f5bad7b0dff4567a498d833a9ba4a49cadf7 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 | 29 +++++++-- src/fold.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++ src/mark.c | 26 +++++++- src/proto/fold.pro | 1 + src/proto/mark.pro | 1 + src/testdir/test_fold.vim | 21 +++++++ 6 files changed, 219 insertions(+), 7 deletions(-) diff --git a/src/ex_cmds.c b/src/ex_cmds.c index f988f5f36..0adb4ebbe 100644 --- a/src/ex_cmds.c +++ b/src/ex_cmds.c @@ -841,24 +841,41 @@ 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 + win_T *win; + tabpage_T *tp; + + FOR_ALL_TAB_WINDOWS(tp, win) { + if (win->w_buffer == curbuf) + foldSwapRange(&win->w_folds, line1, line2, dest + 1, + dest + num_lines); + } +#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 + win_T *win; + tabpage_T *tp; + + FOR_ALL_TAB_WINDOWS(tp, win) { + if (win->w_buffer == curbuf) + foldSwapRange(&win->w_folds, dest + 1, line1 - 1, line1, 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 d75e93700..acb1f4102 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 foldSwapRange_internal(garray_T *gap, linenr_T ftop, linenr_T fbot, linenr_T stop, linenr_T sbot); 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); } + void +foldSwapRange(garray_T *gap, linenr_T firsttop, linenr_T firstbot, + linenr_T secondtop, linenr_T secondbot) +{ + foldSwapRange_internal(gap, firsttop, firstbot, secondtop, secondbot); +} /* Internal functions for "fold_T" {{{1 */ /* cloneFoldGrowArray() {{{2 */ /* @@ -2962,6 +2969,147 @@ 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, *tmp; + + for (; start < end; start++, end--) { + tmp = left = (fold_T *)gap->ga_data + start; + right = (fold_T *)gap->ga_data + end; + *left = *right; + *right = *tmp; + } +} + +/* foldSwapRange_internal() {{{2 */ +/* + * Swap folds within the range "top1" and "bot1" with folds in the range "top2" + * and "bot2". + * Each range is inclusive. + * + * For each range, check for these situations. + * 1 2 3 + * 1 2 3 + * top 2 3 4 5 + * 2 3 4 5 + * bot 2 3 4 5 + * 3 5 6 + * 3 5 6 + * + * 1. not changed + * 2. truncate above "top" + * 3. truncate above "top" + * 4. move to other range. + * 5. truncate at "bot", and then move to other range. + * 6. not changed. + * + * After both ranges of folds have been swapped, we then reorder the fold array + * to ensure it is always ordered. + */ + static void +foldSwapRange_internal(garray_T *gap, linenr_T firsttop, linenr_T firstbot, + linenr_T secondtop, linenr_T secondbot) +{ + linenr_T range_pos[4] = { firsttop, firstbot, secondtop, secondbot }; + linenr_T shift = secondtop - firsttop; + int fold_splits[4] = { 0, 0, 0, 0 }; + int i, j, r, next; + int common_val; + int new_splits[4]; + int truncate_top, truncate_bottom; + int fold_index; + fold_T *fp; + + /* + * Ensure that all folds are truncated as specified in the comment above, + * and, as a side effect populate fold_splits[] with the indexes of the folds + * as specified below: + * 0 first fold that starts at or below firsttop + * 1 first fold that starts below firstbot + * 2 first fold that starts at or below secondtop + * 3 first fold that starts below secondtop + */ + for (r = 0; r < 4;) { + linenr_T line1 = range_pos[r++]; + linenr_T line2 = range_pos[r++]; + + i = r; + + truncate_top = foldFind(gap, line1 - 1, &fp) + && fp->fd_top + fp->fd_len > line1; + fold_splits[i] = (fp - (fold_T *)gap->ga_data); + if (fp < (fold_T *)gap->ga_data + gap->ga_len + && fp->fd_top < line1) + fold_splits[i] += 1; + + if (truncate_top) { + /* If "fp" finishes at line1 - 1, this does nothing. */ + foldRemove(&fp->fd_nested, line1 - fp->fd_top, MAXLNUM); + fp->fd_len = line1 - fp->fd_top; + } + + i += 1; + + truncate_bottom = foldFind(gap, line2 + 1, &fp) && fp->fd_top <= line2; + fold_splits[i] = (fp - (fold_T *)gap->ga_data) + truncate_bottom; + if (truncate_bottom) { + foldRemove(&fp->fd_nested, line2 - fp->fd_top + 1, MAXLNUM); + fp->fd_len = line2 - fp->fd_top + 1; + } + } + + for (j = 1; j < 4; j += 2) { + for (fold_index = fold_splits[j - 1]; + fold_index < fold_splits[j]; fold_index++) + { + fp = (fold_T *)gap->ga_data + fold_index; + if (j > 2) + fp->fd_top -= shift; + else + fp->fd_top += shift; + } + } + + /* Our fold array looks something like this + * 1,2,3|4,5|6,7,8|9,0,1|2,3,4 position in gap->ga_data + * |0 |1 |2 |3 fold_splits indexes + * + * In order to swap the fold positions 9,0,1 and 4,5 we do the following: + * reverse everything in the range [0, 3) + * 1,2,3|1,0,9|8,7,6|5,4|2,3,4 + * |0 |1 |2 |3 + * + * reverse everything in the range [0, 0 + 3 - 2) + * 1,2,3|9,0,1|8,7,6|5,4|2,3,4 + * |0 |1 |2 |3 + * + * reverse everything in the range [0 + 3 - 2, 0 + 3 - 1) + * 1,2,3|9,0,1|6,7,8|5,4|2,3,4 + * |0 |1 |2 |3 + * + * reverse everything in the range [0 + 3 - 1, 3) + * 1,2,3|9,0,1|6,7,8|4,5|2,3,4 + * |0 |1 |2 |3 + * + * Account for 0 (avoid underflow) + */ + next = fold_splits[3] ? fold_splits[3] - 1 : 0; + foldReverseOrder(gap, fold_splits[0], next); + + common_val = fold_splits[0] + fold_splits[3]; + + for (i = 0; i < 4; i++) + new_splits[i] = common_val - fold_splits[3 - i]; + + for (i = 0; i < 3; i++) { + next = new_splits[i + 1] ? new_splits[i + 1] - 1 : 0; + foldReverseOrder(gap, new_splits[i], next); + } +} + + /* foldMerge() {{{2 */ /* * Merge two adjacent folds (and the nested ones in them). diff --git a/src/mark.c b/src/mark.c index 59ac01dc5..21f6e45bb 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..d74bcf870 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 foldSwapRange(garray_T *gap, linenr_T ftop, linenr_T fbot, linenr_T stop, linenr_T sbot); 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 3c27f4f70..53545ed54 100644 --- a/src/testdir/test_fold.vim +++ b/src/testdir/test_fold.vim @@ -203,3 +203,24 @@ func Test_update_folds_expr_read() bwipe! set foldmethod& foldexpr& endfunc + +func! Test_move_folds_around() + new + func! PrepIndent(arg) + return [a:arg] + repeat(["\t".a:arg], 5) + endfu + + 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] + 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)')) + bw! +endfunc -- 2.11.0
