From 66ed6297b0f5b6e571f675840388e0956c5deb9d Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Mon, 20 Aug 2018 21:24:30 -0400 Subject: vim-patch:8.0.1441: using ":undo 0" leaves undo in wrong state Problem: Using ":undo 0" leaves undo in wrong state. Solution: Instead of searching for state 1 and go above, just use the start. (Ozaki Kiichi, closes vim/vim#2595) https://github.com/vim/vim/commit/ce46d934af35d0f774be7f996001db03cf0b894a --- src/nvim/testdir/test_undo.vim | 44 ++++++++++++ src/nvim/undo.c | 154 ++++++++++++++++++++++------------------- 2 files changed, 127 insertions(+), 71 deletions(-) (limited to 'src') diff --git a/src/nvim/testdir/test_undo.vim b/src/nvim/testdir/test_undo.vim index f31499607b..83ede1dc37 100644 --- a/src/nvim/testdir/test_undo.vim +++ b/src/nvim/testdir/test_undo.vim @@ -390,3 +390,47 @@ funct Test_undofile() set undodir& endfunc + +func Test_undo_0() + new + set ul=100 + normal i1 + undo + normal i2 + undo + normal i3 + + undo 0 + let d = undotree() + call assert_equal('', getline(1)) + call assert_equal(0, d.seq_cur) + + redo + let d = undotree() + call assert_equal('3', getline(1)) + call assert_equal(3, d.seq_cur) + + undo 2 + undo 0 + let d = undotree() + call assert_equal('', getline(1)) + call assert_equal(0, d.seq_cur) + + redo + let d = undotree() + call assert_equal('2', getline(1)) + call assert_equal(2, d.seq_cur) + + undo 1 + undo 0 + let d = undotree() + call assert_equal('', getline(1)) + call assert_equal(0, d.seq_cur) + + redo + let d = undotree() + call assert_equal('1', getline(1)) + call assert_equal(1, d.seq_cur) + + bwipe! +endfunc diff --git a/src/nvim/undo.c b/src/nvim/undo.c index f4eb50b3b5..4fdcc991d9 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -1820,7 +1820,7 @@ void undo_time(long step, int sec, int file, int absolute) long closest_start; long closest_seq = 0; long val; - u_header_T *uhp; + u_header_T *uhp = NULL; u_header_T *last; int mark; int nomark; @@ -1842,13 +1842,7 @@ void undo_time(long step, int sec, int file, int absolute) /* "target" is the node below which we want to be. * Init "closest" to a value we can't reach. */ if (absolute) { - if (step == 0) { - // target 0 does not exist, got to 1 and above it. - target = 1; - above = true; - } else { - target = step; - } + target = step; closest = -1; } else { if (dosec) { @@ -1906,6 +1900,11 @@ void undo_time(long step, int sec, int file, int absolute) closest_start = closest; closest_seq = curbuf->b_u_seq_cur; + // When "target" is 0; Back to origin. + if (target == 0) { + goto found; + } + /* * May do this twice: * 1. Search for "target", update "closest" to the best match found. @@ -2021,8 +2020,9 @@ void undo_time(long step, int sec, int file, int absolute) above = TRUE; /* stop above the header */ } +found: /* If we found it: Follow the path to go to where we want to be. */ - if (uhp != NULL) { + if (uhp != NULL || target == 0) { /* * First go up the tree as much as needed. */ @@ -2035,83 +2035,95 @@ void undo_time(long step, int sec, int file, int absolute) uhp = curbuf->b_u_newhead; else uhp = uhp->uh_next.ptr; - if (uhp == NULL || uhp->uh_walk != mark + if (uhp == NULL || (target > 0 && uhp->uh_walk != mark) || (uhp->uh_seq == target && !above)) break; curbuf->b_u_curhead = uhp; u_undoredo(true, true); - uhp->uh_walk = nomark; // don't go back down here + if (target > 0) { + uhp->uh_walk = nomark; // don't go back down here + } } - /* - * And now go down the tree (redo), branching off where needed. - */ - while (!got_int) { - /* Do the change warning now, for the same reason as above. */ - change_warning(0); - - uhp = curbuf->b_u_curhead; - if (uhp == NULL) - break; + // When back to origin, redo is not needed. + if (target > 0) { + // And now go down the tree (redo), branching off where needed. + while (!got_int) { + // Do the change warning now, for the same reason as above. + change_warning(0); - /* Go back to the first branch with a mark. */ - while (uhp->uh_alt_prev.ptr != NULL - && uhp->uh_alt_prev.ptr->uh_walk == mark) - uhp = uhp->uh_alt_prev.ptr; + uhp = curbuf->b_u_curhead; + if (uhp == NULL) { + break; + } - /* Find the last branch with a mark, that's the one. */ - last = uhp; - while (last->uh_alt_next.ptr != NULL - && last->uh_alt_next.ptr->uh_walk == mark) - last = last->uh_alt_next.ptr; - if (last != uhp) { - /* Make the used branch the first entry in the list of - * alternatives to make "u" and CTRL-R take this branch. */ - while (uhp->uh_alt_prev.ptr != NULL) + // Go back to the first branch with a mark. + while (uhp->uh_alt_prev.ptr != NULL + && uhp->uh_alt_prev.ptr->uh_walk == mark) { uhp = uhp->uh_alt_prev.ptr; - if (last->uh_alt_next.ptr != NULL) - last->uh_alt_next.ptr->uh_alt_prev.ptr = - last->uh_alt_prev.ptr; - last->uh_alt_prev.ptr->uh_alt_next.ptr = last->uh_alt_next.ptr; - last->uh_alt_prev.ptr = NULL; - last->uh_alt_next.ptr = uhp; - uhp->uh_alt_prev.ptr = last; - - if (curbuf->b_u_oldhead == uhp) - curbuf->b_u_oldhead = last; - uhp = last; - if (uhp->uh_next.ptr != NULL) - uhp->uh_next.ptr->uh_prev.ptr = uhp; - } - curbuf->b_u_curhead = uhp; + } + + // Find the last branch with a mark, that's the one. + last = uhp; + while (last->uh_alt_next.ptr != NULL + && last->uh_alt_next.ptr->uh_walk == mark) { + last = last->uh_alt_next.ptr; + } + if (last != uhp) { + // Make the used branch the first entry in the list of + // alternatives to make "u" and CTRL-R take this branch. + while (uhp->uh_alt_prev.ptr != NULL) { + uhp = uhp->uh_alt_prev.ptr; + } + if (last->uh_alt_next.ptr != NULL) { + last->uh_alt_next.ptr->uh_alt_prev.ptr = last->uh_alt_prev.ptr; + } + last->uh_alt_prev.ptr->uh_alt_next.ptr = last->uh_alt_next.ptr; + last->uh_alt_prev.ptr = NULL; + last->uh_alt_next.ptr = uhp; + uhp->uh_alt_prev.ptr = last; - if (uhp->uh_walk != mark) - break; /* must have reached the target */ + if (curbuf->b_u_oldhead == uhp) { + curbuf->b_u_oldhead = last; + } + uhp = last; + if (uhp->uh_next.ptr != NULL) { + uhp->uh_next.ptr->uh_prev.ptr = uhp; + } + } + curbuf->b_u_curhead = uhp; - /* Stop when going backwards in time and didn't find the exact - * header we were looking for. */ - if (uhp->uh_seq == target && above) { - curbuf->b_u_seq_cur = target - 1; - break; - } + if (uhp->uh_walk != mark) { + break; // must have reached the target + } + + // Stop when going backwards in time and didn't find the exact + // header we were looking for. + if (uhp->uh_seq == target && above) { + curbuf->b_u_seq_cur = target - 1; + break; + } - u_undoredo(false, true); + u_undoredo(false, true); - /* Advance "curhead" to below the header we last used. If it - * becomes NULL then we need to set "newhead" to this leaf. */ - if (uhp->uh_prev.ptr == NULL) - curbuf->b_u_newhead = uhp; - curbuf->b_u_curhead = uhp->uh_prev.ptr; - did_undo = FALSE; + // Advance "curhead" to below the header we last used. If it + // becomes NULL then we need to set "newhead" to this leaf. + if (uhp->uh_prev.ptr == NULL) { + curbuf->b_u_newhead = uhp; + } + curbuf->b_u_curhead = uhp->uh_prev.ptr; + did_undo = false; - if (uhp->uh_seq == target) /* found it! */ - break; + if (uhp->uh_seq == target) { // found it! + break; + } - uhp = uhp->uh_prev.ptr; - if (uhp == NULL || uhp->uh_walk != mark) { - // Need to redo more but can't find it... - internal_error("undo_time()"); - break; + uhp = uhp->uh_prev.ptr; + if (uhp == NULL || uhp->uh_walk != mark) { + // Need to redo more but can't find it... + internal_error("undo_time()"); + break; + } } } } -- cgit From 19717ca1e8da17890b25789285c759bc1d57a12d Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Mon, 20 Aug 2018 23:06:47 -0400 Subject: undo: above,did_undo in undo_time() are bool --- src/nvim/undo.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/nvim/undo.c b/src/nvim/undo.c index 4fdcc991d9..a9940a53c7 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -1827,8 +1827,8 @@ void undo_time(long step, int sec, int file, int absolute) int round; int dosec = sec; int dofile = file; - int above = FALSE; - int did_undo = TRUE; + bool above = false; + bool did_undo = true; /* First make sure the current undoable change is synced. */ if (curbuf->b_u_synced == false) @@ -2016,8 +2016,9 @@ void undo_time(long step, int sec, int file, int absolute) target = closest_seq; dosec = FALSE; dofile = FALSE; - if (step < 0) - above = TRUE; /* stop above the header */ + if (step < 0) { + above = true; // stop above the header + } } found: -- cgit From 6f55aa3502823fa0b20e3cc8055a3f3f3f4e5153 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Mon, 20 Aug 2018 23:36:36 -0400 Subject: undo: update undo_time() function signature sec,file,absolute (params) are bool. Fix callers and local variables of undo_time(). --- src/nvim/ex_docmd.c | 18 +++++++++--------- src/nvim/undo.c | 30 ++++++++++++++---------------- 2 files changed, 23 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 20c188ed55..b40cf3d3f7 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -7578,7 +7578,7 @@ static void ex_bang(exarg_T *eap) static void ex_undo(exarg_T *eap) { if (eap->addr_count == 1) /* :undo 123 */ - undo_time(eap->line2, FALSE, FALSE, TRUE); + undo_time(eap->line2, false, false, true); else u_undo(1); } @@ -7613,8 +7613,8 @@ static void ex_redo(exarg_T *eap) static void ex_later(exarg_T *eap) { long count = 0; - int sec = FALSE; - int file = FALSE; + bool sec = false; + bool file = false; char_u *p = eap->arg; if (*p == NUL) @@ -7622,11 +7622,11 @@ static void ex_later(exarg_T *eap) else if (isdigit(*p)) { count = getdigits_long(&p); switch (*p) { - case 's': ++p; sec = TRUE; break; - case 'm': ++p; sec = TRUE; count *= 60; break; - case 'h': ++p; sec = TRUE; count *= 60 * 60; break; - case 'd': ++p; sec = TRUE; count *= 24 * 60 * 60; break; - case 'f': ++p; file = TRUE; break; + case 's': ++p; sec = true; break; + case 'm': ++p; sec = true; count *= 60; break; + case 'h': ++p; sec = true; count *= 60 * 60; break; + case 'd': ++p; sec = true; count *= 24 * 60 * 60; break; + case 'f': ++p; file = true; break; } } @@ -7634,7 +7634,7 @@ static void ex_later(exarg_T *eap) EMSG2(_(e_invarg2), eap->arg); else undo_time(eap->cmdidx == CMD_earlier ? -count : count, - sec, file, FALSE); + sec, file, false); } /* diff --git a/src/nvim/undo.c b/src/nvim/undo.c index a9940a53c7..35b54f0a13 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -1804,16 +1804,14 @@ static void u_doit(int startcount, bool quiet, bool do_buf_event) u_undo_end(undo_undoes, false, quiet); } -/* - * Undo or redo over the timeline. - * When "step" is negative go back in time, otherwise goes forward in time. - * When "sec" is FALSE make "step" steps, when "sec" is TRUE use "step" as - * seconds. - * When "file" is TRUE use "step" as a number of file writes. - * When "absolute" is TRUE use "step" as the sequence number to jump to. - * "sec" must be FALSE then. - */ -void undo_time(long step, int sec, int file, int absolute) +// Undo or redo over the timeline. +// When "step" is negative go back in time, otherwise goes forward in time. +// When "sec" is false make "step" steps, when "sec" is true use "step" as +// seconds. +// When "file" is true use "step" as a number of file writes. +// When "absolute" is true use "step" as the sequence number to jump to. +// "sec" must be false then. +void undo_time(long step, bool sec, bool file, bool absolute) { long target; long closest; @@ -1825,8 +1823,8 @@ void undo_time(long step, int sec, int file, int absolute) int mark; int nomark; int round; - int dosec = sec; - int dofile = file; + bool dosec = sec; + bool dofile = file; bool above = false; bool did_undo = true; @@ -1867,7 +1865,7 @@ void undo_time(long step, int sec, int file, int absolute) if (target <= 0) /* Go to before first write: before the oldest change. Use * the sequence number for that. */ - dofile = FALSE; + dofile = false; } else { /* Moving forward to a newer write. */ target = curbuf->b_u_save_nr_cur + step; @@ -1875,7 +1873,7 @@ void undo_time(long step, int sec, int file, int absolute) /* Go to after last write: after the latest change. Use * the sequence number for that. */ target = curbuf->b_u_seq_last + 1; - dofile = FALSE; + dofile = false; } } } else @@ -2014,8 +2012,8 @@ void undo_time(long step, int sec, int file, int absolute) } target = closest_seq; - dosec = FALSE; - dofile = FALSE; + dosec = false; + dofile = false; if (step < 0) { above = true; // stop above the header } -- cgit From 0ff4854800b1a5cd45829d1dc6cfaaae308f3a1b Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Mon, 20 Aug 2018 23:42:37 -0400 Subject: undo: did_undo,absolute in u_undo_end() are bool --- src/nvim/undo.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/nvim/undo.c b/src/nvim/undo.c index 35b54f0a13..d8322e4aa6 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -2386,8 +2386,8 @@ static void u_undoredo(int undo, bool do_buf_event) /// Otherwise, report the number of changes (this may be incorrect /// in some cases, but it's better than nothing). static void u_undo_end( - int did_undo, ///< just did an undo - int absolute, ///< used ":undo N" + bool did_undo, ///< just did an undo + bool absolute, ///< used ":undo N" bool quiet) { char *msgstr; @@ -2427,7 +2427,7 @@ static void u_undo_end( /* For ":undo N" we prefer a "after #N" message. */ if (absolute && curbuf->b_u_curhead->uh_next.ptr != NULL) { uhp = curbuf->b_u_curhead->uh_next.ptr; - did_undo = FALSE; + did_undo = false; } else if (did_undo) uhp = curbuf->b_u_curhead; else -- cgit From 612f3fd57a94ce9674650a973f6bc7a5dfed1949 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Mon, 20 Aug 2018 23:43:49 -0400 Subject: undo: undo_undoes is bool --- src/nvim/undo.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/nvim/undo.c b/src/nvim/undo.c index d8322e4aa6..6982e440c5 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -122,7 +122,7 @@ static long u_newcount, u_oldcount; * When 'u' flag included in 'cpoptions', we behave like vi. Need to remember * the action that "u" should do. */ -static int undo_undoes = FALSE; +static bool undo_undoes = false; static int lastmark = 0; @@ -591,7 +591,7 @@ int u_savecommon(linenr_T top, linenr_T bot, linenr_T newbot, int reload) uep->ue_next = curbuf->b_u_newhead->uh_entry; curbuf->b_u_newhead->uh_entry = uep; curbuf->b_u_synced = false; - undo_undoes = FALSE; + undo_undoes = false; #ifdef U_DEBUG u_check(FALSE); @@ -1676,7 +1676,7 @@ void u_undo(int count) } if (vim_strchr(p_cpo, CPO_UNDO) == NULL) - undo_undoes = TRUE; + undo_undoes = true; else undo_undoes = !undo_undoes; u_doit(count, false, true); -- cgit From ee51061b8c00eb1ac8ce628ba3831236b327ead2 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Mon, 20 Aug 2018 21:51:01 -0400 Subject: lint --- src/nvim/ex_docmd.c | 7 ++++--- src/nvim/undo.c | 25 ++++++++++++++----------- 2 files changed, 18 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index b40cf3d3f7..b22f799da0 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -7577,10 +7577,11 @@ static void ex_bang(exarg_T *eap) */ static void ex_undo(exarg_T *eap) { - if (eap->addr_count == 1) /* :undo 123 */ + if (eap->addr_count == 1) { // :undo 123 undo_time(eap->line2, false, false, true); - else + } else { u_undo(1); + } } static void ex_wundo(exarg_T *eap) @@ -7634,7 +7635,7 @@ static void ex_later(exarg_T *eap) EMSG2(_(e_invarg2), eap->arg); else undo_time(eap->cmdidx == CMD_earlier ? -count : count, - sec, file, false); + sec, file, false); } /* diff --git a/src/nvim/undo.c b/src/nvim/undo.c index 6982e440c5..e15b9ec796 100644 --- a/src/nvim/undo.c +++ b/src/nvim/undo.c @@ -1675,10 +1675,11 @@ void u_undo(int count) count = 1; } - if (vim_strchr(p_cpo, CPO_UNDO) == NULL) + if (vim_strchr(p_cpo, CPO_UNDO) == NULL) { undo_undoes = true; - else + } else { undo_undoes = !undo_undoes; + } u_doit(count, false, true); } @@ -2020,11 +2021,9 @@ void undo_time(long step, bool sec, bool file, bool absolute) } found: - /* If we found it: Follow the path to go to where we want to be. */ + // If we found it: Follow the path to go to where we want to be. if (uhp != NULL || target == 0) { - /* - * First go up the tree as much as needed. - */ + // First go up the tree as much as needed. while (!got_int) { /* Do the change warning now, for the same reason as above. */ change_warning(0); @@ -2034,9 +2033,11 @@ found: uhp = curbuf->b_u_newhead; else uhp = uhp->uh_next.ptr; - if (uhp == NULL || (target > 0 && uhp->uh_walk != mark) - || (uhp->uh_seq == target && !above)) + if (uhp == NULL + || (target > 0 && uhp->uh_walk != mark) + || (uhp->uh_seq == target && !above)) { break; + } curbuf->b_u_curhead = uhp; u_undoredo(true, true); if (target > 0) { @@ -2428,12 +2429,14 @@ static void u_undo_end( if (absolute && curbuf->b_u_curhead->uh_next.ptr != NULL) { uhp = curbuf->b_u_curhead->uh_next.ptr; did_undo = false; - } else if (did_undo) + } else if (did_undo) { uhp = curbuf->b_u_curhead; - else + } else { uhp = curbuf->b_u_curhead->uh_next.ptr; - } else + } + } else { uhp = curbuf->b_u_newhead; + } if (uhp == NULL) *msgbuf = NUL; -- cgit