Timoses / vim-venu

Simple menu for vim
20 stars 2 forks source link

Global callbacks aren't allowed to start with lowercase letters #13

Open ozars opened 5 years ago

ozars commented 5 years ago

Hello again,

I realized that vim doesn't allow function callbacks starting with lowercase letter in the global (g:) scope a while ago. I realized this while I was trying to use custom menu printing callbacks.

Just a few solutions coming into my mind:

I've been planning for reporting this and discussing possible solutions, but I couldn't find a chance. Instead, I applied some quick patches to my forked branch (following second option). It works for me for now but it has diverged from main repo and I don't want to maintain a fork in the long term.

So, I decided not to delay reporting this even if I don't have time to discuss/apply fixes. Although I'd still prefer second option, any other solution would probably work for me. I'd appreciate if you could address this issue whenever you have some time. Thanks!

Timoses commented 5 years ago

Could you give some examples (code) on

so I can take a look?

Moving all configurations under venu# namespace. I'm not sure if this requires still starting with uppercase letter though. It shouldn't, as there are already functions starting with lowercase in venu# namespace.

Why is this necessary? I assume this is what you did with the following (L241 in https://github.com/Timoses/vim-venu/compare/master...ozars:callback-feat)

- function! s:print(name, itemsOrMenus) abort
+ function! venu#defaultPrintCallback(name, itemsOrMenus) abort

I've been planning for reporting this and discussing possible solutions, but I couldn't find a chance. [...] So, I decided not to delay reporting this even if I don't have time to discuss/apply fixes.

Very much appreciated!

ozars commented 5 years ago

I apologize for late response. I have been dealing with various stuff on multiple fronts.

* where "Global callbacks aren't allowed to start with lowercase letters" to reproduce the behaviour and

Assigning some function to any configuration callback, e.g. g:venu_print_callback, produces error.

* usage of second option (config dictionary)

An example from my vimrc:

https://github.com/ozars/dotfiles/blob/98af59b513fdbf6ac525922bcc9cb6a9d059393f/vimrc#L636-L638

Possible options for conf are PrintCallback, SelectCallback, FormatEntryCallback and StartVenuCallback.

Why is this necessary? I assume this is what you did with the following (L241 in master...ozars:callback-feat)

This isn't necessary actually. It's just one possible way to work around lowercase letter callback problem (e.g. renaming g:venu_print_callback to venu#printCallback). I did it to expose venu#defaultPrintCallback as default printing function provided by the library. So, my intention was unrelated to this issue (but see the next paragraph). Just as a random example, user may want to make some custom filtering for printed menu items in this step, so they can just wrap default printing callback.

The way you understood it wasn't my intention at all, but after understanding how you interpreted it now, it didn't look like a bad idea. Happy accident :-) This way actually there wouldn't be any need for maintaining separate configuration options as long as these plug points are well-documented and kept stable. If user wants to overwrite some behavior they can just overwrite default callbacks with their own. If you'd like to take this route, then removing "default" prefix from those callbacks may be reasonable.