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
👍 Add popup
module to privide a compatibility layer for pupup in Vim and floating window in Neovim
#236
Conversation
WalkthroughThe recent changes focus on refining window and popup management in Neovim and Vim. A reintroduced Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (12)
- function/nvim/_generated.ts (1 hunks)
- function/nvim/_manual.ts (1 hunks)
- function/nvim/nvim_open_win.ts (1 hunks)
- function/nvim/nvim_win_get_config.ts (1 hunks)
- function/vim/_generated.ts (1 hunks)
- function/vim/_manual.ts (1 hunks)
- function/vim/popup_create.ts (1 hunks)
- popup/mod.ts (1 hunks)
- popup/mod_test.ts (1 hunks)
- popup/nvim.ts (1 hunks)
- popup/types.ts (1 hunks)
- popup/vim.ts (1 hunks)
Files not reviewed due to errors (1)
- api_functions.vim (no review received)
Files skipped from review due to trivial changes (1)
- function/vim/_generated.ts
Additional comments: 14
function/vim/_manual.ts (1)
- 1-2: The export statements for
"prop_add_list.ts"
and"popup_create.ts"
are correctly added and follow TypeScript's module export patterns. Ensure that the referenced modules are correctly implemented and exist within the project structure.function/nvim/_manual.ts (1)
- 1-2: The export statements for
"nvim_open_win.ts"
and"nvim_win_get_config.ts"
are correctly added and follow TypeScript's module export patterns. Ensure that the referenced modules are correctly implemented and exist within the project structure.function/nvim/nvim_win_get_config.ts (2)
- 17-26: The function
nvim_win_get_config
is correctly implemented with clear documentation on its purpose and usage. It properly uses TypeScript features for typing and promises, ensuring compatibility with Neovim's API for window configurations.- 28-39: The interface
NvimGetConfigResult
is well-defined, extendingNvimOpenWinConfig
with modifications to therelative
property. This is consistent with Neovim's API expectations and demonstrates good use of TypeScript'sOmit
utility type.popup/nvim.ts (4)
- 10-29: The
ensurePrerequisites
function is cleverly implemented to inject necessary Vim script functions into Neovim, usingulid
to ensure uniqueness. This approach effectively prepares the environment for popup window management.- 31-45: The
openPopup
function is correctly implemented, effectively using async/await patterns and interacting with Neovim's API to open popup windows. The conversion of options to Neovim's expected parameters is clear and well-handled.- 47-49: The
closePopup
function is straightforward and correctly uses Neovim's API to close popup windows. This is a simple yet essential part of managing popup windows in Neovim.- 51-69: Utility functions
toNvimOpenWinConfig
,toNvimBorder
, andtoNvimWinhighlight
are well-implemented, providing clear and effective conversions from TypeScript options to Neovim's expected parameters.popup/vim.ts (3)
- 7-16: The
openPopup
function is correctly implemented for Vim, effectively using async/await patterns and interacting with Vim's API to open popup windows. The conversion of options to Vim's expected parameters is clear and well-handled.- 18-20: The
closePopup
function is straightforward and correctly uses Vim's API to close popup windows. This is a simple yet essential part of managing popup windows in Vim.- 22-51: Utility functions
toPopupCreateOptions
,posFromAnchor
, andtoVimBorder
are well-implemented, providing clear and effective conversions from TypeScript options to Vim's expected parameters. The use ofunreachable
for enhanced type safety in unexpected cases is a good practice.function/vim/popup_create.ts (2)
- 35-45: The
popup_create
function is overloaded to accept specific arguments or a spread ofunknown[]
. This design ensures flexibility but also requires careful validation of arguments when using the spread syntax to prevent runtime errors.- 425-462: The
PopupProp
interface is specific to text properties within a popup. It's a good practice to separate concerns by defining such an interface, which enhances code readability and maintainability. Ensure that the usage of this interface is clear in the context of popups, especially how developers are expected to use it with thePopupCreateWhat
type.function/nvim/nvim_open_win.ts (1)
- 143-153: The
nvim_open_win
function, similar topopup_create
in the Vim module, is overloaded to accept specific arguments or a spread ofunknown[]
. This approach provides flexibility in function usage. Ensure that argument validation is robust, especially for the spread syntax case, to prevent runtime errors.
16ff46d
to
11c0c4c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #236 +/- ##
==========================================
+ Coverage 83.00% 83.56% +0.56%
==========================================
Files 48 55 +7
Lines 2677 2903 +226
Branches 232 262 +30
==========================================
+ Hits 2222 2426 +204
- Misses 455 475 +20
- Partials 0 2 +2 ☔ View full report in Codecov by Sentry. |
797c4cd
to
85ad739
Compare
85ad739
to
fcf8ab9
Compare
fcf8ab9
to
fa4c32a
Compare
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.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files selected for processing (12)
- function/nvim/_generated.ts (1 hunks)
- function/nvim/_manual.ts (1 hunks)
- function/nvim/nvim_open_win.ts (1 hunks)
- function/nvim/nvim_win_get_config.ts (1 hunks)
- function/vim/_generated.ts (1 hunks)
- function/vim/_manual.ts (1 hunks)
- function/vim/popup_create.ts (1 hunks)
- popup/mod.ts (1 hunks)
- popup/mod_test.ts (1 hunks)
- popup/nvim.ts (1 hunks)
- popup/types.ts (1 hunks)
- popup/vim.ts (1 hunks)
Files not reviewed due to errors (1)
- nvim_api_functions.vim (no review received)
Files skipped from review due to trivial changes (2)
- function/nvim/_manual.ts
- function/vim/_manual.ts
Additional comments: 33
function/nvim/nvim_win_get_config.ts (1)
- 28-40: The definition of
NvimGetConfigResult
interface is clear and enhances type safety by specifying the exact string literals for therelative
property. This is a good practice for ensuring that the code is used correctly in the context of Neovim's API.popup/nvim.ts (4)
- 10-26: The
ensurePrerequisites
function efficiently checks and sets up the necessary Vim script for opening popups in Neovim, using a unique identifier to avoid conflicts. This is a good practice for managing dynamic script injection in a Vim/Neovim environment.- 29-43: The
openPopup
function is well-implemented, correctly handling the asynchronous setup of prerequisites and using utility functions to convert options to Neovim's API format. This modular approach enhances code readability and maintainability.- 45-47: The
closePopup
function is correctly implemented, using Neovim's API to close popup windows. This straightforward approach is appropriate and effective.- 49-97: The utility functions
toNvimOpenWinConfig
,toNvimBorder
, andtoNvimWinhighlight
are well-implemented, correctly converting popup options to Neovim's API format. This modular approach enhances code readability and maintainability.popup/vim.ts (3)
- 7-16: The
openPopup
function is well-implemented, correctly handling the conversion of options to Vim's API format and opening popup windows. This modular approach enhances code readability and maintainability.- 18-20: The
closePopup
function is correctly implemented, using Vim's API to close popup windows. This straightforward approach is appropriate and effective.- 22-101: The utility functions
toPopupCreateOptions
,posFromAnchor
, andtoVimBorder
are well-implemented, correctly converting popup options to Vim's API format. This modular approach enhances code readability and maintainability.popup/types.ts (1)
- 1-194: The types and interfaces defined in this file are clear, comprehensive, and well-documented, providing a solid foundation for the popup module's configuration options. This enhances the usability and maintainability of the module.
popup/mod.ts (1)
- 135-169: The
open
function in the popup module is well-implemented, providing a unified API for opening popup windows in both Vim and Neovim. The error handling and conditional logic based on the host environment are correctly applied. The re-export of types fromtypes.ts
enhances the module's usability.function/vim/popup_create.ts (1)
- 35-45: The implementation of
popup_create
function correctly utilizes Denops' capabilities for calling Vim script functions. Ensure that when using the spread syntax with...args
, the types of arguments passed match the expected types for the corresponding Vim script function to avoid runtime errors.function/vim/_generated.ts (22)
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The file contains a large number of Vim script function definitions. Each function will be reviewed individually to ensure correctness, adherence to best practices, and other key areas.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_beeps
is correctly defined for its intended purpose of testing Vim functionality related to beeps or visual bells. The usage ofv:errors
for error handling in tests is consistent with common practices.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_equal
is well-defined for its purpose of comparing values in tests and allowing for customizable error messages. The implementation follows common testing practices.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_equalfile
is correctly implemented for comparing the contents of two files in tests. It provides clear and useful error messages for test failures.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_exception
is well-designed for asserting exceptions in tests, usingv:exception
andv:errors
consistently with Vim script testing practices.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_fails
is correctly implemented for testing commands that are expected to fail, with support for specific error checking. This is useful for ensuring that the correct error is produced in tests.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_false
is straightforward and correctly implemented for asserting false conditions in tests. The documentation is clear and concise.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_inrange
is well-defined for asserting value ranges in tests, providing clear error messages when the value is out of range.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_match
is correctly implemented for asserting pattern matches in tests, with clear documentation and usage examples.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_nobeep
is correctly implemented for testing commands that should not produce a beep or visual bell, with clear documentation.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_notequal
is well-defined for asserting that two values are not equal in tests, with support for customizable error messages.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_notmatch
is correctly implemented for asserting that a value does not match a specific pattern in tests, with clear documentation and usage examples.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_report
is straightforward and correctly implemented for manually reporting test failures, with clear documentation.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
assert_true
is correctly implemented for asserting true conditions in tests, with clear documentation.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_add
is well-defined for adding text properties to buffer lines, with a flexibleprops
dictionary for specifying text property attributes.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_clear
is correctly implemented for clearing text properties from buffer lines, with an optionalprops
dictionary for specifying the buffer.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_find
is well-designed for searching text properties based on various criteria, with a detailed return value for found properties.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_list
is correctly implemented for listing text properties in buffer lines, with support for filtering by type and ID. The return value format is appropriate for the function's purpose.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_remove
is well-defined for removing text properties based on match criteria, with aprops
dictionary allowing for flexible specification of target properties.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_type_add
is correctly implemented for adding new text property types with customizable attributes, using aprops
dictionary for flexible specification.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_type_change
is well-designed for modifying attributes of existing text property types, using aprops
dictionary for flexible updates.
- 2383-2388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The function
prop_type_delete
is correctly implemented for removing text property types by name, with an optionalprops
dictionary for buffer-specific operations.
Using Regular Async/Await:
Developers can open an unforcusable floating window (popup) using the following TypeScript code:
Using
await using
Statement:Alternatively, developers can utilize the
await using
statement to achieve the same result:Summary by CodeRabbit