fix(jumplist): Ctrl+o, Ctrl+i weird behavior when deleting buffers #25461

Problem:
- Navigation is not always symmetric: pressing Ctrl+o n times followed
  by Ctrl+i n times does not always gets me back to where I started.
- Invalid buffers are not skipped by Ctrl+i/o, I have to press Ctrl+i/o
  multiple times to get to the next/previous buffer.

Solution:
- Remove all entries of a buffer from the jump list when deleting it.
- Don't add a new entry to the jump list if the next buffer to be
  displayed is already in the jump list.

Closes #25365
This commit is contained in:
vE5li 2024-01-29 04:08:51 +01:00 committed by GitHub
parent eee52d3427
commit af6537bc66
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 209 additions and 36 deletions

View File

@ -1183,6 +1183,32 @@ static int empty_curbuf(bool close_others, int forceit, int action)
return retval;
}
/// Remove every jump list entry referring to a given buffer.
/// This function will also adjust the current jump list index.
void buf_remove_from_jumplist(buf_T *deleted_buf)
{
// Remove all jump list entries that match the deleted buffer.
for (int i = curwin->w_jumplistlen - 1; i >= 0; i--) {
buf_T *buf = buflist_findnr(curwin->w_jumplist[i].fmark.fnum);
if (buf == deleted_buf) {
// Found an entry that we want to delete.
curwin->w_jumplistlen -= 1;
// If the current jump list index behind the entry we want to
// delete, move it back by one.
if (curwin->w_jumplistidx > i && curwin->w_jumplistidx > 0) {
curwin->w_jumplistidx -= 1;
}
// Actually remove the entry from the jump list.
for (int d = i; d < curwin->w_jumplistlen; d++) {
curwin->w_jumplist[d] = curwin->w_jumplist[d + 1];
}
}
}
}
/// Implementation of the commands for the buffer list.
///
/// action == DOBUF_GOTO go to specified buffer
@ -1205,6 +1231,7 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
{
buf_T *buf;
buf_T *bp;
bool update_jumplist = true;
bool unload = (action == DOBUF_UNLOAD || action == DOBUF_DEL
|| action == DOBUF_WIPE);
@ -1362,7 +1389,11 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
// If the buffer to be deleted is not the current one, delete it here.
if (buf != curbuf) {
// Remove the buffer to be deleted from the jump list.
buf_remove_from_jumplist(buf);
close_windows(buf, false);
if (buf != curbuf && bufref_valid(&bufref) && buf->b_nwindows <= 0) {
close_buffer(NULL, buf, action, false, false);
}
@ -1382,40 +1413,53 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
if (au_new_curbuf.br_buf != NULL && bufref_valid(&au_new_curbuf)) {
buf = au_new_curbuf.br_buf;
} else if (curwin->w_jumplistlen > 0) {
int jumpidx = curwin->w_jumplistidx - 1;
if (jumpidx < 0) {
jumpidx = curwin->w_jumplistlen - 1;
}
// Remove the current buffer from the jump list.
buf_remove_from_jumplist(curbuf);
forward = jumpidx;
while (jumpidx != curwin->w_jumplistidx) {
buf = buflist_findnr(curwin->w_jumplist[jumpidx].fmark.fnum);
if (buf != NULL) {
// Skip current and unlisted bufs. Also skip a quickfix
// buffer, it might be deleted soon.
if (buf == curbuf || !buf->b_p_bl || bt_quickfix(buf)) {
buf = NULL;
} else if (buf->b_ml.ml_mfp == NULL) {
// skip unloaded buf, but may keep it for later
if (bp == NULL) {
bp = buf;
// It's possible that we removed all jump list entries, in that case we need to try another
// approach
if (curwin->w_jumplistlen > 0) {
// If the index is the same as the length, the current position was not yet added to the jump
// list. So we can safely go back to the last entry and search from there.
if (curwin->w_jumplistidx == curwin->w_jumplistlen) {
curwin->w_jumplistidx = curwin->w_jumplistlen - 1;
}
int jumpidx = curwin->w_jumplistidx;
forward = jumpidx;
do {
buf = buflist_findnr(curwin->w_jumplist[jumpidx].fmark.fnum);
if (buf != NULL) {
// Skip unlisted bufs. Also skip a quickfix
// buffer, it might be deleted soon.
if (!buf->b_p_bl || bt_quickfix(buf)) {
buf = NULL;
} else if (buf->b_ml.ml_mfp == NULL) {
// skip unloaded buf, but may keep it for later
if (bp == NULL) {
bp = buf;
}
buf = NULL;
}
buf = NULL;
}
}
if (buf != NULL) { // found a valid buffer: stop searching
break;
}
// advance to older entry in jump list
if (!jumpidx && curwin->w_jumplistidx == curwin->w_jumplistlen) {
break;
}
if (--jumpidx < 0) {
jumpidx = curwin->w_jumplistlen - 1;
}
if (jumpidx == forward) { // List exhausted for sure
break;
}
if (buf != NULL) { // found a valid buffer: stop searching
curwin->w_jumplistidx = jumpidx;
update_jumplist = false;
break;
}
// advance to older entry in jump list
if (!jumpidx && curwin->w_jumplistidx == curwin->w_jumplistlen) {
break;
}
if (--jumpidx < 0) {
jumpidx = curwin->w_jumplistlen - 1;
}
if (jumpidx == forward) { // List exhausted for sure
break;
}
} while (jumpidx != curwin->w_jumplistidx);
}
}
@ -1511,7 +1555,7 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
}
// Go to the other buffer.
set_curbuf(buf, action);
set_curbuf(buf, action, update_jumplist);
if (action == DOBUF_SPLIT) {
RESET_BINDING(curwin); // reset 'scrollbind' and 'cursorbind'
@ -1533,14 +1577,17 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
/// DOBUF_UNLOAD unload it
/// DOBUF_DEL delete it
/// DOBUF_WIPE wipe it out
void set_curbuf(buf_T *buf, int action)
void set_curbuf(buf_T *buf, int action, bool update_jumplist)
{
buf_T *prevbuf;
int unload = (action == DOBUF_UNLOAD || action == DOBUF_DEL
|| action == DOBUF_WIPE);
OptInt old_tw = curbuf->b_p_tw;
setpcmark();
if (update_jumplist) {
setpcmark();
}
if ((cmdmod.cmod_flags & CMOD_KEEPALT) == 0) {
curwin->w_alt_fnum = curbuf->b_fnum; // remember alternate file
}
@ -3675,7 +3722,7 @@ void ex_buffer_all(exarg_T *eap)
// Open the buffer in this window.
swap_exists_action = SEA_DIALOG;
set_curbuf(buf, DOBUF_GOTO);
set_curbuf(buf, DOBUF_GOTO, false);
if (!bufref_valid(&bufref)) {
// Autocommands deleted the buffer.
swap_exists_action = SEA_NONE;

View File

@ -405,7 +405,7 @@ buf_found:
// Open the changed buffer in the current window.
if (buf != curbuf) {
set_curbuf(buf, unload ? DOBUF_UNLOAD : DOBUF_GOTO);
set_curbuf(buf, unload ? DOBUF_UNLOAD : DOBUF_GOTO, true);
}
theend:

View File

@ -3,6 +3,7 @@ local Screen = require('test.functional.ui.screen')
local clear = helpers.clear
local command = helpers.command
local dedent = helpers.dedent
local eq = helpers.eq
local fn = helpers.fn
local feed = helpers.feed
@ -192,6 +193,131 @@ describe("jumpoptions=stack behaves like 'tagstack'", function()
end)
end)
describe('buffer deletion', function()
local base_file = 'Xtest-functional-buffer-deletion'
local file1 = base_file .. '1'
local file2 = base_file .. '2'
local file3 = base_file .. '3'
local base_content = 'text'
local content1 = base_content .. '1'
local content2 = base_content .. '2'
local content3 = base_content .. '3'
local function format_jumplist(input)
return dedent(input)
:gsub('%{file1%}', file1)
:gsub('%{file2%}', file2)
:gsub('%{file3%}', file3)
:gsub('%{content1%}', content1)
:gsub('%{content2%}', content2)
:gsub('%{content3%}', content3)
end
before_each(function()
clear()
command('clearjumps')
write_file(file1, content1, false, false)
write_file(file2, content2, false, false)
write_file(file3, content3, false, false)
command('edit ' .. file1)
command('edit ' .. file2)
command('edit ' .. file3)
end)
it('deletes jump list entries when the current buffer is deleted', function()
command('edit ' .. file1)
eq(
format_jumplist([[
jump line col file/text
3 1 0 {content1}
2 1 0 {file2}
1 1 0 {file3}
>]]),
exec_capture('jumps')
)
command('bwipeout')
eq(
format_jumplist([[
jump line col file/text
1 1 0 {file2}
> 0 1 0 {content3}]]),
exec_capture('jumps')
)
end)
it('deletes jump list entries when another buffer is deleted', function()
eq(
format_jumplist([[
jump line col file/text
2 1 0 {file1}
1 1 0 {file2}
>]]),
exec_capture('jumps')
)
command('bwipeout ' .. file2)
eq(
format_jumplist([[
jump line col file/text
1 1 0 {file1}
>]]),
exec_capture('jumps')
)
end)
it('sets the correct jump index when the current buffer is deleted', function()
feed('<C-O>')
eq(
format_jumplist([[
jump line col file/text
1 1 0 {file1}
> 0 1 0 {content2}
1 1 0 {file3}]]),
exec_capture('jumps')
)
command('bw')
eq(
format_jumplist([[
jump line col file/text
1 1 0 {file1}
> 0 1 0 {content3}]]),
exec_capture('jumps')
)
end)
it('sets the correct jump index when the another buffer is deleted', function()
feed('<C-O>')
eq(
format_jumplist([[
jump line col file/text
1 1 0 {file1}
> 0 1 0 {content2}
1 1 0 {file3}]]),
exec_capture('jumps')
)
command('bwipeout ' .. file1)
eq(
format_jumplist([[
jump line col file/text
> 0 1 0 {content2}
1 1 0 {file3}]]),
exec_capture('jumps')
)
end)
end)
describe('jumpoptions=view', function()
local file1 = 'Xtestfile-functional-editor-jumps'
local file2 = 'Xtestfile-functional-editor-jumps-2'