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

Improve window options configurations #305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Townk
Copy link

@Townk Townk commented Jun 3, 2024

Changes Description

This commit adds the ability to customize window options vim.wo for windows Overseer creates (e.g. the task_list, the output, etc.). It also fixes a problem where pre-defined window options are reset after the main buffer changes on the window.

@github-actions github-actions bot requested a review from stevearc June 3, 2024 22:02
relativenumber = false,
foldcolumn = "0",
signcolumn = "no",
statuscolumn = "",
Copy link
Author

Choose a reason for hiding this comment

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

I added the statuscolumn option here and in other places because simply turning off the other column options won't clean the "side gutter" for folks using it. That being said, I think this option was introduced on NeoVim 0.9, and since Overseer supports NeoVim 0.8, I can remove the option from here if you prefer. Just let me know.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it makes sense to add it, but we should add a check in util.set_window_opts so that it doesn't crash on neovim 0.8

@Townk Townk changed the title Improove window options configurations Improve window options configurations Jun 4, 2024
## Changes Description

This commit adds the ability to customize window options `vim.wo` for
windows Overseer creates (e.g. the task_list, the output, etc.). It also
fixes a problem where pre-defined window options are reset after the
main buffer changes on the window.
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 the PR! Generally on board with the approach. Left some comments, and I've also refactored some of this logic recently so you'll need to resolve the merge conflicts.

relativenumber = false,
foldcolumn = "0",
signcolumn = "no",
statuscolumn = "",
Copy link
Owner

Choose a reason for hiding this comment

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

I think it makes sense to add it, but we should add a check in util.set_window_opts so that it doesn't crash on neovim 0.8

Comment on lines +68 to +71
-- When the Task List is positioned at the bottom, it will show its output
-- window on the right side of the split. The next configuration table
-- tweaks options for such window
output = {
Copy link
Owner

Choose a reason for hiding this comment

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

Hmmm...putting the config here feels duplicative of the task_win key. Here's what I think makes the most sense:

  • repurpose task_win to be for all task output windows, not just the floating ones
  • put the current task_win options under task_win.float to indicate that the configuration is for a subset of the task output windows that are floating
  • add some shims to config.setup to maintain backwards compatibility

Comment on lines +226 to +232
-- The documentation of `vim.api.nvim_win_set_buf` states that it will
-- "Set the current buffer in a window, without side effects", but
-- unfortunately, some window options are not kept when a "new" buffer is
-- set to the window. For instance, the `winhighlight` and the experimental
-- `statuscolumn` options may change with a new buffer, therefore, to make
-- the appearance of the window consistent, we re-apply its window options
-- when switching its current buffer.
Copy link
Owner

Choose a reason for hiding this comment

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

This behavior is explained if you read :help local-options. Maybe "without side effects" is a misleading claim in general, because there are a lot of things that can happen as a result of that call (autocmds, for instance).

vim.api.nvim_win_set_width(0, layout.calculate_width(nil, config.task_list))
if direction == "bottom" then
vim.api.nvim_win_set_height(0, layout.calculate_height(nil, config.task_list))
end
-- Set the filetype only after we enter the buffer so that FileType autocmds
-- behave properly
vim.bo[bufnr].filetype = "OverseerList"
vim.api.nvim_set_option_value("filetype", "OverseerList", { buf = bufnr })
Copy link
Owner

Choose a reason for hiding this comment

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

Why this change? The only reason I was using nvim_set_option_value for window options is because vim.wo updates the local and global option value.

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.

2 participants