Skip to content
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

fix: set alternate when using floating windows (#285) #526

Merged
merged 1 commit into from
Dec 21, 2024

Conversation

iovis
Copy link
Contributor

@iovis iovis commented Dec 2, 2024

This ensures the alternate is set even when using open_float(), I copied the code from restore_alt_buf, so I'm sure there could be refactor opportunities. Let me know your thoughts!

Fixes: #285

@github-actions github-actions bot requested a review from stevearc December 2, 2024 18:51
@stevearc
Copy link
Owner

stevearc commented Dec 8, 2024

Everything related to the alternate buffer is very tricky. Please add at least one test to cover this case in https://github.com/stevearc/oil.nvim/blob/master/tests/altbuf_spec.lua. Make sure that it fails without your change applied and passes with it.

@iovis
Copy link
Contributor Author

iovis commented Dec 9, 2024

I completely missed the tests, my bad. I'll give it a go tonight!

@iovis iovis force-pushed the open_float_alternate branch from cbb4dd1 to f6314c6 Compare December 9, 2024 05:34
@iovis
Copy link
Contributor Author

iovis commented Dec 9, 2024

Done, sorry for missing the tests! I confirmed the new test fails in master but passes on my branch.

Copy link
Owner

@stevearc stevearc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the test! I fiddled with it a bit on my end, and I think that a simpler fix is to remove the keepalt = true on this line.

keepalt = true,

That handles the float win case for me, and doesn't seem to impact any of the other tests.

@iovis iovis force-pushed the open_float_alternate branch from f6314c6 to bbb102f Compare December 10, 2024 20:07
@github-actions github-actions bot requested a review from stevearc December 10, 2024 20:07
@iovis
Copy link
Contributor Author

iovis commented Dec 10, 2024

Thanks for adding the test! I fiddled with it a bit on my end, and I think that a simpler fix is to remove the keepalt = true on this line.

You're right, I can confirm it works on my end and it looks like a much more appropriate fix. I just sent the revised code!

@stevearc
Copy link
Owner

Something strange going on with the test failures in 0.8. I'll investigate when I get a chance

@iovis iovis force-pushed the open_float_alternate branch from bbb102f to 7c1f27b Compare December 19, 2024 04:47
@iovis
Copy link
Contributor Author

iovis commented Dec 19, 2024

I think I found the issue in v0.8, when I removed keepalt completely, then this table:

-- lua/oil/init.lua:717
local mods = {                  
  vertical = opts.vertical,     
  horizontal = opts.horizontal, 
  split = opts.split,           
}                               

could result in an empty table, which apparently neovim v0.8 doesn't like:

Error executing vim.schedule lua callback: ~/code/vim/oil.nvim/lua/oil/init.lua:745: 'mods' must be a Dictionary
stack traceback:
        [C]: in function 'cmd'
        ~/code/vim/oil.nvim/lua/oil/init.lua:745: in function 'callback'
        ~/code/vim/oil.nvim/lua/oil/adapters/files.lua:260: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>

Changing it from removing it completely to keepalt = false resolves the issue and the tests still pass (which I cleaned up a bit).

@stevearc
Copy link
Owner

Great, thanks for the fix!

@stevearc stevearc merged commit c5f7c56 into stevearc:master Dec 21, 2024
9 checks passed
@iovis iovis deleted the open_float_alternate branch December 21, 2024 04:21
@tjex
Copy link

tjex commented Dec 21, 2024

Thank you so much for this everyone!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: alternate not set with open_float()
3 participants