Skip to content

Conversation

@MahdiNazemi
Copy link

@MahdiNazemi MahdiNazemi commented May 5, 2023

When using context.vim, it is important that the number of lines to scroll takes account of the context height. This pull request aims to solve this issue.

The fix is based on wellle/context.vim#128.

As this is my first contribution to a Vim plugin, please feel free to modify the code to adhere to your preferred style guide.

@cskeeters
Copy link
Owner

Thanks for looking into this. context.vim looks like a cool plugin. I haven't tested your fix but I'll assume that it works for this discussion. Since this change would make it so that it would work regardless of if context.vim is being used, my first thought is to incorporate it. On the other hand, if other plugins also tried to do similar things, there would have to be special cases for each one which could result in fragile plugins. Perhaps some thought and inquiry are appropriate.

Which context presenter is being used? Does my plugin need to be modified when the preview or popup presenter is used?

@MahdiNazemi
Copy link
Author

MahdiNazemi commented May 8, 2023

Thanks for taking the time to look into this pull request.

I agree that finding a solution that works for all plugins would be most beneficial. One possible solution would be to allow users to provide offsets to the center, top, and bottom functions, enabling them to define their own mappings based on their plugin usage. For context.vim, one would call the appropriate functions to find the offset and pass that value to one of the aforesaid functions.

Regarding context.vim, it appears to override the zt and H mappings due to the context preview at the top of the page. This preview alters the number of visible code lines to the user by effectively hiding certain lines of code.

@cskeeters
Copy link
Owner

Which context presenter are you using?

@MahdiNazemi
Copy link
Author

Based on the documentation, vim-popup should be set as the default option given that I use Vim 9. A sample image of what I see is included below as reference (also sourced from the docs):

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