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 variable to control AutoTrigger #1551

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

jdujava
Copy link
Contributor

@jdujava jdujava commented Jan 26, 2024

Now it is possible to disable AutoTrigger in the current buffer by setting the buffer variable b:UltiSnipsAutoTrigger = 0.

Now it is possible to disable AutoTrigger via global variable g:UltiSnipsAutoTrigger, and to dynamically toggle AutoTrigger on/off with Ultisnips#ToggleAutoTrigger().

This is just a first crude shot at implementing this "toggling feature" (documentation and so on TODO).
What do you think, is this alright, or is there perhaps some better way in your opinion?

@SirVer
Copy link
Owner

SirVer commented Feb 2, 2024

I worry a bit about performance. Calling out to vim.eval is slow and this code will run on every cursor movement. This should probably be checked in vimscript before calling into python. Overall, I am fine with the feature.

@jdujava jdujava changed the title Add buffer variable to control AutoTrigger Add variable to control AutoTrigger Feb 2, 2024
@ces42
Copy link
Contributor

ces42 commented Feb 2, 2024

I worry a bit about performance. Calling out to vim.eval is slow and this code will run on every cursor movement. This should probably be checked in vimscript before calling into python. Overall, I am fine with the feature.

Yes this can be done much better by removing/re-creating the UltiSnips_AutoTrigger autogroup from plugin/UltiSnips.vim using vimscript. There's no need to write python code for this.

@jdujava
Copy link
Contributor Author

jdujava commented Feb 2, 2024

To not complicate things, I added only a global toggling mechanism.

@ces42 To be honest, I wasn't sure wheter UltiSnips_Manager._track_change() was needed also for other purposes than AutoTrigger, so I tried it this way. But I am certainly open to other (possibly better) solutions!

@ces42
Copy link
Contributor

ces42 commented Feb 3, 2024

Adding this to autoload/UltiSnips.vim provides control over autotrigger:

function! UltiSnips#EnableAutotrigger() abort
    augroup UltiSnips_AutoTrigger
        au!
        au InsertCharPre * call UltiSnips#TrackChange()
        au TextChangedI * call UltiSnips#TrackChange()
        if exists('##TextChangedP')
            au TextChangedP * call UltiSnips#TrackChange()
        endif
    augroup END
endfunction

function! UltiSnips#DisableAutotrigger() abort
    augroup UltiSnips_AutoTrigger
        au!
    augroup END
endfunction

function! UltiSnips#ToggleAutotrigger() abort
    " if exists('#UltiSnips_AutoTrigger')
    if exists('#UltiSnips_AutoTrigger#TextChangedI')
        call UltiSnips#DisableAutotrigger()
    else
        call UltiSnips#EnableAutotrigger()
    endif
endfunction

EDIT: apparently the augroup doesn't get deleted when you do au!, only it's autocommands. So you need to replace exists('#UltiSnips_AutoTrigger') with exists('#UltiSnips_AutoTrigger#TextChangedI').

@SirVer
Copy link
Owner

SirVer commented Feb 10, 2024

Great, but a test is still missing. Will you work on that @jdujava ?

Now it is possible to disable AutoTrigger via global variable `g:UltiSnipsAutoTrigger`,
and to dynamically toggle AutoTrigger on/off with `Ultisnips#ToggleAutoTrigger()`.
@jdujava
Copy link
Contributor Author

jdujava commented Feb 11, 2024

@SirVer Added tests and documentation, does it look OK?

@jdujava
Copy link
Contributor Author

jdujava commented Mar 11, 2024

@SirVer is it good to merge, or is there perhaps anything else to do?

@SirVer
Copy link
Owner

SirVer commented Mar 15, 2024

@jdujava Yes, it looks great, an exemplary contribution to an open source project. Thank you very much! Sorry that I took so long to get around to it.

@SirVer SirVer merged commit 808114c into SirVer:master Mar 15, 2024
@jdujava jdujava deleted the autotrigger_bufvariable branch March 15, 2024 19:32
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.

3 participants