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

feat: add win_options to preview_win #514

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

mehalter
Copy link
Contributor

This adds the ability to customize the window options for the preview window for files/directories. This takes a very basic approach, outside of just adding the win_options field to the lua type annotations it makes sure to set the win_options when creating the previewwindow windows. The other wrinkle it accounts for is to make sure to apply the preview_win.win_options after applying oil win_options if it's within a previewwindow. This makes sure it gets the necessary settings such as conceal and concealcursor while also applying the settings that the user wants for preview windows.

Let me know what you think of this approach!

Resolves #509

@github-actions github-actions bot requested a review from stevearc November 11, 2024 19:59
@mehalter
Copy link
Contributor Author

Hm, not sure what happened with the "failed" test. I checked the logs and it seems to be passing and locally it seems to pass

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.

Hmmm...not sure what's going on with the tests. Does it pass for you locally? I've sometimes seen some finicky behavior like this if there are timeouts in an async test.

edit: ah okay yes it does pass locally. Not sure...try rebasing (which you'll have to do for merge conflicts anyway)

@@ -323,6 +323,7 @@ local M = {}

---@class (exact) oil.PreviewWindowConfig
---@field update_on_cursor_moved boolean
---@field win_options? table<string, any>
Copy link
Owner

Choose a reason for hiding this comment

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

You'll want this one to omit the ? and you'll need to also add this to oil.SetupPreviewWindowConfig. This is the internal reference for type safety, the other one is for typing the options passed to setup(). It's annoying to keep them both in sync, but it's the only way I could think of to get good type safety (and LuaLS completions) in both locations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah that makes sense, thanks for the guidance here. I updated the commit appropriately

@mehalter mehalter force-pushed the preview_win_win_opts branch from 24890f8 to 9cebd6d Compare November 12, 2024 18:10
@github-actions github-actions bot requested a review from stevearc November 12, 2024 18:11
@mehalter mehalter force-pushed the preview_win_win_opts branch from 9cebd6d to c96f1b4 Compare November 12, 2024 18:12
@mehalter
Copy link
Contributor Author

It looks like something is causing it to hang after the tests all pass. I'm not sure what it would be doing after all the tests are done. I'm taking a deeper look now at how the tests are set up. I don't think anything is happening in this PR to make things get funky 🤔

@mehalter
Copy link
Contributor Author

mehalter commented Nov 12, 2024

Ah, it looks like something is causing the win_options_spec.lua test file to just completely hang This is incorrect. I'm looking more now

EDIT: this is coming from the altbuf_spec.lua file

@mehalter mehalter force-pushed the preview_win_win_opts branch from c96f1b4 to 7011a9b Compare November 12, 2024 18:21
@mehalter
Copy link
Contributor Author

Ah, I found the problem @stevearc it was causing problems because I was a dingus and forgot to update the default preview_win.win_options so it was failing to get setup correctly. Should be good now! Sorry for all the noise!

@stevearc
Copy link
Owner

Thanks for the PR!

@stevearc stevearc merged commit bbeed86 into stevearc:master Nov 12, 2024
9 checks passed
@mehalter mehalter deleted the preview_win_win_opts branch November 12, 2024 18:46
@jamilraichouni
Copy link

jamilraichouni commented Nov 13, 2024

Hi @mehalter !

Thanks for this cool contribution!

One question wrt this PR:

I set

require("oil").setup({
    preview_win = {
        win_options = {
            foldenable = false,
        },
    }
})

and it works as long as the preview buffer has no focus and I stay in the oil buffer with the list of dir contents.
But when I make the preview buffer the current buffer my previewed code is immediately folded and the win option is overwritten by whatever setting I have globally for the filetype.
Any idea how to avoid that?
I often use the preview to yank stuff I want to put into some other files. In that scenario it is not helpful, that a disabled folding gets enabled when I want to yank/ put code.

Thanks,
Jamil

@mehalter
Copy link
Contributor Author

I'm not sure, just want to remove whatever is in your configuration that is setting foldenabled when entering a buffer

@kah-ell
Copy link

kah-ell commented Nov 14, 2024

Maybe noautocmd and eventignore can get you towards your goal.

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.

feature request: Extend preview.win_options to override win_options
4 participants