Repository navigation
Reuse opened windows #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
dc1fd94
3768d28
247c6d5
faf9f5e
c3b44c9
c7a1db3
fceba62
c02ef33
1ba0541
b82c2f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,4 +36,16 @@ M.sorted_global_marks = function() | |
| return marks | ||
| end | ||
|
|
||
| M.set_cursor_for_mark_in_window = function(window_id, mark) | ||
| if window_id ~= 0 then | ||
| vim.api.nvim_set_current_win(window_id) | ||
| end | ||
| vim.api.nvim_win_set_cursor(window_id, { mark.pos[2], mark.pos[3] }) | ||
|
bvdmitri marked this conversation as resolved.
Resolved
|
||
| end | ||
|
|
||
| M.set_cursor_for_mark_in_current_window = function(mark) | ||
| vim.cmd("silent buffer " .. mark.file) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I'm actually no longer sure why I think I would rather inline this logic into the only call-site because you already run a check on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it was necessary in case if the |
||
| M.set_cursor_for_mark_in_window(0, mark) | ||
| end | ||
|
|
||
| return M | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,20 +131,27 @@ describe("Recall", function() | |
| place_cursor(unpack(c_pos)) | ||
| recall.toggle() | ||
|
|
||
| recall.goto_prev() | ||
| assert.are.same(b_pos, vim.api.nvim_win_get_cursor(0)) | ||
| for _, reuse_opened_windows in ipairs({ true, false }) do | ||
| require('recall.navigation').opts.reuse_opened_windows = reuse_opened_windows | ||
|
|
||
| recall.goto_prev() | ||
| assert.are.same(a_pos, vim.api.nvim_win_get_cursor(0)) | ||
| recall.goto_prev() | ||
| assert.are.same(b_pos, vim.api.nvim_win_get_cursor(0)) | ||
|
|
||
| recall.goto_next() | ||
| assert.are.same(b_pos, vim.api.nvim_win_get_cursor(0)) | ||
| recall.goto_prev() | ||
| assert.are.same(a_pos, vim.api.nvim_win_get_cursor(0)) | ||
|
|
||
| recall.goto_next() | ||
| assert.are.same(c_pos, vim.api.nvim_win_get_cursor(0)) | ||
| recall.goto_next() | ||
| assert.are.same(b_pos, vim.api.nvim_win_get_cursor(0)) | ||
|
|
||
| recall.goto_next() | ||
| assert.are.same(c_pos, vim.api.nvim_win_get_cursor(0)) | ||
|
|
||
| recall.goto_prev() | ||
| assert.are.same(b_pos, vim.api.nvim_win_get_cursor(0)) | ||
| recall.goto_prev() | ||
| assert.are.same(b_pos, vim.api.nvim_win_get_cursor(0)) | ||
|
|
||
| recall.goto_next() | ||
| assert.are.same(c_pos, vim.api.nvim_win_get_cursor(0)) | ||
| end | ||
| end) | ||
|
|
||
| it("can clear all marks", function() | ||
|
|
@@ -204,4 +211,55 @@ describe("Recall", function() | |
| assert.are.equal(results[3].lnum, 20) | ||
| assert.are.equal(results[3].col, 0) | ||
| end) | ||
|
|
||
| it("reuses opened windows when reuse_opened_windows is enabled", function() | ||
| -- Create a second buffer for a second window | ||
| local bufnr2 = vim.api.nvim_create_buf(true, false) | ||
| set_lines(bufnr2, line_count) | ||
| local temp_path2 = luv.os_tmpdir() .. "/nvim-recall-test2-" .. luv.hrtime() .. ".txt" | ||
| if jit and jit.os == "OSX" then | ||
| temp_path2 = "/private" .. temp_path2 | ||
| end | ||
| table.insert(temp_paths, temp_path2) | ||
| vim.api.nvim_buf_set_name(bufnr2, temp_path2) | ||
| vim.api.nvim_buf_set_option(bufnr2, "modified", false) | ||
| vim.cmd("w") | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I find the readability of this test setup a bit rough. The comment
Suggestion: make a function that returns just what you need for this test, and add "why" comments there.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This piece of code is pretty much a copy-paste of the
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, let's maybe put it into a function so that it's not repeated in multiple places. I can try to understand it at some other point but it should at least not be repeated multiple times.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's already repeated in multiple places then ignore this 😅
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I will make it a simple function! |
||
|
|
||
| recall.setup({ reuse_opened_windows = true }) | ||
|
|
||
| vim.api.nvim_set_current_buf(bufnr) | ||
| local win1 = vim.api.nvim_get_current_win() | ||
| local win2 = vim.api.nvim_open_win(bufnr2, true, { split = 'right' }) | ||
| assert.are.not_equal(win1, win2) | ||
|
|
||
| vim.api.nvim_set_current_win(win1) | ||
| place_cursor(10, 1) | ||
| recall.mark() | ||
|
|
||
| vim.api.nvim_set_current_win(win2) | ||
| place_cursor(20, 1) | ||
| recall.mark() | ||
|
|
||
| vim.api.nvim_set_current_win(win2) | ||
| place_cursor(20, 1) | ||
| recall.goto_next() | ||
|
|
||
| local current_win = vim.api.nvim_get_current_win() | ||
| local current_buf = vim.api.nvim_get_current_buf() | ||
| local cursor_pos = vim.api.nvim_win_get_cursor(current_win) | ||
|
|
||
| assert.are.equal(current_win, win1, "Should reuse the window already displaying the target buffer") | ||
| assert.are.equal(current_buf, bufnr) | ||
| assert.are.same(cursor_pos, { 10, 1 }) | ||
|
|
||
| recall.goto_next() | ||
|
|
||
| current_win = vim.api.nvim_get_current_win() | ||
| current_buf = vim.api.nvim_get_current_buf() | ||
| cursor_pos = vim.api.nvim_win_get_cursor(current_win) | ||
|
|
||
| assert.are.equal(current_win, win2, "Should reuse the window already displaying the target buffer") | ||
| assert.are.equal(current_buf, bufnr2) | ||
| assert.are.same(cursor_pos, { 20, 1 }) | ||
| end) | ||
| end) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate the intention of keeping backward compatibility, but I feel like this is more of a bug fix than an optional enhancement. I would probably not make this an option but make
reuse_opened_windows = truethe default, without an option to configure that. If that breaks someone's workflow, they can open an issue.Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we make it a bug fix this can become
1.2.1instead.