lifepillar / vim-mucomplete

Chained completion that works the way you want!
MIT License
913 stars 18 forks source link

Check s:N exists before cycling #35

Closed lacygoill closed 7 years ago

lacygoill commented 7 years ago

Hello,

If I hit C-x C-p C-l at the end of this line:

License: This file

I have the following error:

    Error detected while processing function
    mucomplete#cycle[2]..<SNR>66_next_method:
    line    1:
    E121: Undefined variable: s:N
    Error detected while processing function
    mucomplete#cycle[2]..<SNR>66_next_method:
    line    1:
    E15: Invalid expression: (s:cycle ? (s:i + s:dir + s:N) % s:N : s:i + s:dir)

The bug occurs when you type a key to move backward OR forward in the chain, without having invoked a method in the chain at least once before.

Initially, I thought the solution was to initialize s:N in cycle(), exactly as it was defined in complete(). But then, I realized that it wasn't a good idea to let the user call s:next_method(), without having entered the chain at least once.

If they've never entered the chain, they have no position inside it. So, there's no reference point on which base our relative motion in the chain. IOW, it doesn't make sense to try and support this weird / edge case.

Maybe the best solution is to prevent s:next_method() to be called when the user never invoked a method in the chain.

How do we know whether they invoked a method? If they did, the mucomplete#complete() function was invoked at least once. It it was, it must have created the variable s:N. Besides, s:N is only created inside mucomplete#complete(), nowhere else. It means there's an equivalence between the existence of this variable and the user having invoked a method at least once.

So, to fix this bug, inside mucomplete#cycle() we could test the existence of s:N before invoking s:next_method().

lifepillar commented 7 years ago

Thanks for reporting this. I'll examine your suggestion thoroughly as soon as I can.

lifepillar commented 7 years ago

I think the proper way to fix this is to let s:N = 0 as part of the initialization of the script's status, which amounts to having an empty completion chain. When a completion chain is empty, s:next_method() just returns an empty string.

lifepillar commented 7 years ago

This is fixed in the current master the way I have outlined above. Feel free to reopen this if necessary.

Bug hunters like you help making this plugin better and better. Thanks!