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

Allow to customize options for floating windows #300

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

lervag
Copy link
Contributor

@lervag lervag commented Nov 4, 2024

This is a first attempt at addressing #299; it is probably not perfect and may (will) need some work.

Let me know what you think!

Copy link
Owner

@MagicDuck MagicDuck left a comment

Choose a reason for hiding this comment

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

@lervag Thank you for your contribution! 😄
Please see my comments for some minor changes that should be done.

Ideally should also add some tests to tests/base/test_ui.lua where you override some option like the border and assert that the screenshot is correct. Tests use mini.test. Check out https://github.com/MagicDuck/grug-far.nvim/blob/main/CONTRIBUTING.md for how to run them via the command line.

lua/grug-far/opts.lua Outdated Show resolved Hide resolved
lua/grug-far/actions/help.lua Outdated Show resolved Hide resolved
lua/grug-far/opts.lua Outdated Show resolved Hide resolved
lua/grug-far/opts.lua Outdated Show resolved Hide resolved
@lervag lervag force-pushed the add-window-config branch from ff453d9 to 7d32039 Compare November 5, 2024 21:58
@lervag
Copy link
Contributor Author

lervag commented Nov 5, 2024

@lervag Thank you for your contribution! 😄 Please see my comments for some minor changes that should be done.

Thanks for the good and useful comments; I've updated the code accordingly. I believe it should be quite good now.

Ideally should also add some tests to tests/base/test_ui.lua where you override some option like the border and assert that the screenshot is correct. Tests use mini.test. Check out https://github.com/MagicDuck/grug-far.nvim/blob/main/CONTRIBUTING.md for how to run them via the command line.

Ok, I'll look into this as well when I get the time; probably tomorrow or Thursday.

I made a minimal test right now. Not sure if it is sufficient here, but at least it's something that we can discuss.

lervag added a commit to lervag/grug-far.nvim that referenced this pull request Nov 5, 2024
@lervag lervag force-pushed the add-window-config branch from 7d32039 to 9925882 Compare November 5, 2024 22:15
lervag added a commit to lervag/grug-far.nvim that referenced this pull request Nov 5, 2024
@lervag lervag force-pushed the add-window-config branch from 9925882 to c272db2 Compare November 5, 2024 22:20
@MagicDuck
Copy link
Owner

Looks good! That test you added would be sufficient for the help window case. If you can, add similar tests for the history and preview windows.

@lervag lervag force-pushed the add-window-config branch from c272db2 to e4a3311 Compare November 6, 2024 21:00
@lervag
Copy link
Contributor Author

lervag commented Nov 6, 2024

Ok, I've added additional tests now. Please check if the tests are up to your standards here - the test framework is unfamiliar to me, so I may have misunderstood something.

Copy link
Owner

@MagicDuck MagicDuck left a comment

Choose a reason for hiding this comment

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

looks good! Thanks for patiently implementing all my suggestions! :)

@MagicDuck MagicDuck merged commit 26415d3 into MagicDuck:main Nov 7, 2024
5 checks passed
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