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

Add custom expand options #87

Merged
merged 3 commits into from
Nov 16, 2022
Merged

Conversation

cnrrobertson
Copy link
Contributor

Based on the ideas behind the expand options: i,A,b,w to limit triggering an expansion to certain scenarios, this commit allows for user-defined expand options. The user definition in the setup function should define the option letter as a key and a function to be called with no arguments to determine if expansion is possible.

The custom expand options are currently given priority over i,A,b,w to ensure they are not ignored.

A test for custom expand option (commented line) has been created, but I found it difficult to set up the neovim/vusted/busted environment to verify it. I have however tested it for my use case with latex snippet expansion within math equations and it is working well. #75

cnrrobertson and others added 3 commits November 14, 2022 22:25
Based on the ideas behind the expand options: i,A,b,w to limit
triggering an expansion to certain scenarios, this commit allows for
user-defined expand options. The user definition in the `setup` function
should define the option letter as a key and a function to be called with
no arguments to determine if expansion is possible.

The custom expand options are currently given priority over i,A,b,w to
ensure they are not ignored.

Test for custom expand option (commented line)
@dcampos
Copy link
Owner

dcampos commented Nov 16, 2022

Thanks for the pull request! I've added some commits to fix some CI errors and took the time to also fix the unit tests that were failing. Other than that, it looks good to me, so I'll go ahead and merge it.

In the future it may be a good idea to allow user-defined conditions to override the built-in ones, so that we don't break the user config if we add a new built-in condition.

@dcampos dcampos merged commit eb2f7e6 into dcampos:master Nov 16, 2022
@dcampos dcampos mentioned this pull request Nov 16, 2022
@cnrrobertson
Copy link
Contributor Author

Awesome! Glad it's been merged.

In the future it may be a good idea to allow user-defined conditions to override the built-in ones, so that we don't break the user config if we add a new built-in condition.

That's a good idea. It might be as straightforward as implementing the built-in conditions as the user-defined conditions are now implemented. Currently, all user conditions are checked, then built-in conditions are checked. If they are combined into one step, a new built-in condition wouldn't cause any trouble.

I can try this out and submit a new PR for you to look over if you're interested.

@dcampos
Copy link
Owner

dcampos commented Nov 17, 2022

I can try this out and submit a new PR for you to look over if you're interested.

Sure. Your idea of implementing the built-in ones like the user-defined ones sounds good. The only one which I imagine wouldn't be possible to implement that way is the autotrigger (A), but that could be kept reserved.

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