Import vim native menus to Venu

[ozars]

This is work-in-progress.

Currently, venu#native#list utility method parses output of menu command and returns it as a dictionary.

It probably makes more sense for this dictionary to inherit vemus dictionary structure: https://github.com/Timoses/vim-venu/blob/4897c60d591d45d246373fbf22c32283d6229a19/autoload/venu.vim#L10-L17

I will update it to do so.

[Timoses]

Awesome! I have to find some time to check this out (probably next weekend).

Meanwhile, I've gone a bit crazy today and made the menu a bit more capable. See #4 . Perhaps test your code with that branch as well, to see if anything broke.

Also note that I've removed any preceding _ from properties in the menu dictionary (e.g. _filetypes -> filetypes) as I failed to see its worth (initially it was meant to keep venu-interal properties "private", but as it turned out all those are set by the user via commands).

[ozars]

Meanwhile, I've gone a bit crazy today and made the menu a bit more capable. See #4 . Perhaps test your code with that branch as well, to see if anything broke.

It looks good! I have one suggestion though. A method for unregistering a menu somehow or clearing all menu items may be useful in case of reimports.

[Timoses]

Meanwhile, I've gone a bit crazy today and made the menu a bit more capable. See #4 . Perhaps test your code with that branch as well, to see if anything broke.

It looks good! I have one suggestion though. A method for unregistering a menu somehow or clearing all menu items may be useful in case of reimports.

Added this point to the TODO list in https://github.com/Timoses/vim-venu/pull/4

[Timoses]

Magician with the holy vimregex wand. Nice work!

I think it would make sense to connect it to venu with this PR to actually get some real functionality. Let me know if you need any help connecting it to venu.

[Timoses]

Would it make sense to store the menu imported via your code to some kind of cache? The user can execute something like VenuImportNativeMenu to renew the cache? Then if the user opens vim again the menu will persist over the cache without importing it again.

[ozars]

Commit name and description don't match. If there are fixes to the previous commit, they should be squashed before introducing new functionality in this commit.

I plan to squash everything done on native.vim file into one commit before merge.

Magician with the holy vimregex wand. Nice work!

Haha thanks! :)

I think it would make sense to connect it to venu with this PR to actually get some real functionality. Let me know if you need any help connecting it to venu.

Yeah, it's incomplete currently. I will push a few more commits.

Would it make sense to store the menu imported via your code to some kind of cache? The user can execute something like VenuImportNativeMenu to renew the cache? Then if the user opens vim again the menu will persist over the cache without importing it again.

Caching is a good idea if there is significant performance hit on startup. Is there any caching mechanism vim provides or do we need serializing/deserializing stuff on some file? It may be better if we look at the performance first though before implementing that.

Thanks for the insightful feedback! I will update accordingly. I hope to push some commits in a couple of hours.

[ozars]

@Timoses It's integrated to Venu now and functioning properly AFAICS.

Users may simply call venu#native#import() in their .vimrc to import native menu entries. They can use g:venu_menu_filter to provide their own import filter, otherwise default filter is used. Also by default amenu command is used for importing entries, but this behavior can be modified by setting g:venu_menu_cmd.

Please let me know if you have any suggestions. If everything looks good, I can squash all commits into one.

[Timoses]

I have added below into another issue #6, so I wouldn't add it as a requirement for this PR as to not blow it out of proportion. (If you feel like it go ahead though ; ) ).

---

Another idea: It would be nice to control which menu to import. E.g., if I have the plugin RiV (https://github.com/gu-fan/riv.vim) running, it defines a menu named "Riv". Something like:

--- Menus ---
75 Riv
  500 Project
    500 Index^I<C-E>ww
        n    <Plug>(RivProjectIndex)
        v    <C-C><Plug>(RivProjectIndex)<C-\><C-G>
        s    <C-C><Plug>(RivProjectIndex)<C-\><C-G>
        o    <C-C><Plug>(RivProjectIndex)<C-\><C-G>
        i    <C-\><C-O><Plug>(RivProjectIndex)
        c    <C-C><Plug>(RivProjectIndex)<C-\><C-G>
    500 List^I<C-E>wa
        n    <Plug>(RivProjectList)
...

Now I'd only like to import this menu in ftplugin/rst.vim for reST files where I might write:

call venu#native#import("Riv")

This gets in the way of the first argument (filter), but perhaps it would suffice to have the filter function defined as a global variable?

Another idea: It would be nice to receive a venu menu selectively, so the user can choose where to hang it in her/his venu tree. E.g.:

# following function would return a venu menu which contains only the Riv :menu
let s:riv = venu#native#{???fetch???}("Riv")
# I can then handle s:riv like any other venu menu
call venu#addItem(s:mySpecialGuiMenu, "Riv GUI", s:riv)

[Timoses]

It's amazing work! I've already tested it and successfully imported the menus. Great stuff!

[ozars]

It's amazing work! I've already tested it and successfully imported the menus. Great stuff!

Glad to see it's working and useful! :)

Another idea: It would be nice to control which menu to import. E.g., if I have the plugin RiV (https://github.com/gu-fan/riv.vim) running, it defines a menu named "Riv". Something like:

You can actually do this and even beyond with filtering function. Example:

" a:key is needed though unnecessary, for internally calling this function with vim filter()
function MyVenuFilter(key, menu)
    return a:menu.name == 'Riv' && venu#native#defaultFilter(a:key, a:menu)
endfunction

call venu#native#import(function("MyVenuFilter"))

Ugly one liner with vim 8 lambdas:

call venu#native#import({k, v -> v.name == 'Riv' && venu#native#defaultFilter(k, v)})

venu#native#defaultFilter is useful in removing separators and empty submenus if there are any. That's why I decided to expose it in the public API.

I had thought about a better filtering interface though. In fact, I first attempted to provide users two options for the filter, either a whitelist (of type v:t_list) or a function (of type v:t_func). (There was even a secondary buffer specific filter in addition to global one). Later I decided to strip all these additional features in favor of simplicity. That's because although it's tempting to provide more interface for user for more customized configuration, it can get quite messy easily since different users may (and probably will) have different tastes and needs. Consider these possibilities:

1) A user may want to include just a menu, e.g. "Riv". Simple enough. 2) A user may want to include multiple menus, e.g. "Riv" and "Plugin". Not so complex. 3) A user may want to import just a submenu, e.g. "Riv.Project". Umm.. okay? 4) A user may want to import an arbitrary mix of menus and submenus, e.g. "Riv", "Plugin.PO-Files", "Solarized.Contrast". Uh-oh... Requires some thinking on the structure of configuration parameter and some work on parsing that structure.

It's possible to cover all these possibilities with some work, yet still there can be some other needs not addressed. What if user wants to move a menu to some other level (to top-level perhaps) or match menus by some wildcard/regex or reorder whitelisted menus etc.? That's why I decided to drop the whitelist option altogether. I think it requires a lot of work, meaning possibly breaking some stuff and more code to maintain, for some functionality which could be done by user with some custom filtering function. Maybe I'm over-complicating it, I don't know...

Another idea: It would be nice to receive a venu menu selectively, so the user can choose where to hang it in her/his venu tree. E.g.:

This can be implemented using venu#native#parseMenu and s:CreateVenuFromMenu. Shall I expose the latter in API e.g. venu#native#createVenuFromMenu?

ps. BTW, filtering function allows modifying a:menu parameter since menu dictionary parameter is passed by reference. A useful side-effect.

[ozars]

I made changes as we discussed. Please let me know if you have any other suggestions, otherwise it's ready for merge. I plan to squash all commits.

[Timoses]

So far, lvgtm.

I think it's a great idea to expose venu#native#createVenuFromMenu.

Would you care to write some documentation (e.g. how to use it, examples, etc.)? I'd think perhaps that a separate document would be good (as its additional functionality) which could go into docs/import.md or something? Let me know if you can manage. Otherwise just squash and let me know when I can pull it into master.

[ozars]

I think it's a great idea to expose venu#native#createVenuFromMenu.

Done in the last commit.

Would you care to write some documentation (e.g. how to use it, examples, etc.)? I'd think perhaps that a separate document would be good (as its additional functionality) which could go into docs/import.md or something? Let me know if you can manage. Otherwise just squash and let me know when I can pull it into master.

Sure, I will. I will ping you once I get done and squash all.

[ozars]

@Timoses I included some documentation. Let me know if you have any suggestions. Also feel free to directly modify the branch if you'd like to make any changes.

[Timoses]

I've tried to add a bit to the documentation.

Apart from the "priority" question, lgtm.