From 9b2a3895eedb3c46179b1ed923b0f1214da04c5b Mon Sep 17 00:00:00 2001 From: Ron Yorston Date: Thu, 15 Apr 2021 12:00:50 +0100 Subject: [PATCH] vi: correctly record deleted characters The undo queue didn't record deleted characters properly. For example, insert some text, backspace over a couple of characters then exit insert mode. At this point undo will restore two nulls instead of the deleted characters. The fix is in undo_push(): record the state of the UNDO_USE_SPOS flag and clear it before using 'u_type'. Also, update the comments to reflect the fact that UNDO_QUEUED_FLAG isn't actually used. function old new delta undo_push 443 435 -8 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-8) Total: -8 bytes Signed-off-by: Ron Yorston Signed-off-by: Denys Vlasenko --- editors/vi.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/editors/vi.c b/editors/vi.c index 31b5d3e8a..fef328117 100644 --- a/editors/vi.c +++ b/editors/vi.c @@ -399,26 +399,31 @@ struct globals { #define UNDO_DEL 1 #define UNDO_INS_CHAIN 2 #define UNDO_DEL_CHAIN 3 -// UNDO_*_QUEUED must be equal to UNDO_xxx ORed with UNDO_QUEUED_FLAG -#define UNDO_QUEUED_FLAG 4 +# if ENABLE_FEATURE_VI_UNDO_QUEUE #define UNDO_INS_QUEUED 4 #define UNDO_DEL_QUEUED 5 -#define UNDO_USE_SPOS 32 -#define UNDO_EMPTY 64 +# endif + // Pass-through flags for functions that can be undone #define NO_UNDO 0 #define ALLOW_UNDO 1 #define ALLOW_UNDO_CHAIN 2 # if ENABLE_FEATURE_VI_UNDO_QUEUE #define ALLOW_UNDO_QUEUED 3 - char undo_queue_state; +# else +// If undo queuing disabled, don't invoke the missing queue logic +#define ALLOW_UNDO_QUEUED ALLOW_UNDO +# endif + +# if ENABLE_FEATURE_VI_UNDO_QUEUE +#define UNDO_USE_SPOS 32 +#define UNDO_EMPTY 64 + char undo_queue_state; // One of UNDO_INS, UNDO_DEL, UNDO_EMPTY int undo_q; char *undo_queue_spos; // Start position of queued operation char undo_queue[CONFIG_FEATURE_VI_UNDO_QUEUE_MAX]; -# else -// If undo queuing disabled, don't invoke the missing queue logic -#define ALLOW_UNDO_QUEUED 1 # endif + struct undo_object { struct undo_object *prev; // Linking back avoids list traversal (LIFO) int start; // Offset where the data should be restored/deleted @@ -1445,7 +1450,7 @@ static void yank_status(const char *op, const char *p, int cnt) #endif /* FEATURE_VI_YANKMARK */ #if ENABLE_FEATURE_VI_UNDO -static void undo_push(char *, unsigned, unsigned char); +static void undo_push(char *, unsigned, int); #endif // open a hole in text[] @@ -1572,9 +1577,12 @@ static void flush_undo_data(void) // Undo functions and hooks added by Jody Bruchon (jody@jodybruchon.com) // Add to the undo stack -static void undo_push(char *src, unsigned length, uint8_t u_type) +static void undo_push(char *src, unsigned length, int u_type) { struct undo_object *undo_entry; +# if ENABLE_FEATURE_VI_UNDO_QUEUE + int use_spos = u_type & UNDO_USE_SPOS; +# endif // "u_type" values // UNDO_INS: insertion, undo will remove from buffer @@ -1583,8 +1591,8 @@ static void undo_push(char *src, unsigned length, uint8_t u_type) // The CHAIN operations are for handling multiple operations that the user // performs with a single action, i.e. REPLACE mode or find-and-replace commands // UNDO_{INS,DEL}_QUEUED: If queuing feature is enabled, allow use of the queue - // for the INS/DEL operation. The raw values should be equal to the values of - // UNDO_{INS,DEL} ORed with UNDO_QUEUED_FLAG + // for the INS/DEL operation. + // UNDO_{INS,DEL} ORed with UNDO_USE_SPOS: commit the undo queue # if ENABLE_FEATURE_VI_UNDO_QUEUE // This undo queuing functionality groups multiple character typing or backspaces @@ -1637,9 +1645,7 @@ static void undo_push(char *src, unsigned length, uint8_t u_type) } break; } -# else - // If undo queuing is disabled, ignore the queuing flag entirely - u_type = u_type & ~UNDO_QUEUED_FLAG; + u_type &= ~UNDO_USE_SPOS; # endif // Allocate a new undo object @@ -1656,12 +1662,11 @@ static void undo_push(char *src, unsigned length, uint8_t u_type) } undo_entry->length = length; # if ENABLE_FEATURE_VI_UNDO_QUEUE - if ((u_type & UNDO_USE_SPOS) != 0) { + if (use_spos) { undo_entry->start = undo_queue_spos - text; // use start position from queue } else { undo_entry->start = src - text; // use offset from start of text buffer } - u_type = (u_type & ~UNDO_USE_SPOS); # else undo_entry->start = src - text; # endif