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

[RDY] menu_get() / export menus #6322

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@teto
Contributor

teto commented Mar 21, 2017

adds export_menu that saves menus to a dict.
let l = export_menus("")
With json_encode, this allows UIs to retrieve the menus (likewise for plugins, eg, a certain "palette")

Crashes for now.

@marvim marvim added the WIP label Mar 21, 2017

@teto teto force-pushed the teto:menus branch 2 times, most recently from 0b23039 to 8c196cd Mar 21, 2017

@teto

This comment has been minimized.

Contributor

teto commented Mar 21, 2017

Removed the crash.
In my init.vim, I have:
menu Spell.FR :setlocal spell spelllang=fr_fr<CR>
If you run :echo export_menus(""); it get exported as "ncommand": ":setlocal spell spelllang=fr_fr\r", I wonder if the \r can be a problem ?

@teto teto force-pushed the teto:menus branch 2 times, most recently from fb97fde to 869cea5 Mar 30, 2017

@teto

This comment has been minimized.

Contributor

teto commented Apr 2, 2017

I use a function to test locally:

function! ExportMenus(path, modes)
	" export all for now
	let m = export_menus(a:path, 'n')
	let r =  json_encode(m)
	" put =r " to display in current buffer
	call writefile([r], "menus.txt")
endfunc

So far it generates:

[
    {
        "enabled": 0,
        "hidden": 0,
        "hint": 0,
        "name": "Spell",
        "priority": 500,
        "shortcut": 0,
        "submenus": [
            {
                "enabled": 0,
                "hidden": 0,
                "hint": 0,
                "implementations": [
                    {
                        "noremap": 0,
                        "rhs": ":setlocal spell spelllang=en_us | call histadd('cmd', 'setlocal spell spelllang=en_us')\r",
                        "sid": 0,
                        "silent": 0
                    }
                ],
                "name": "EN_US",
                "priority": 500,
                "shortcut": 0
            },
            {
                "enabled": 0,
                "hidden": 0,
                "hint": 0,
                "implementations": [
                    {
                        "noremap": 0,
                        "rhs": ":setlocal spell spelllang=fr_fr\r",
                        "sid": 0,
                        "silent": 0
                    }
                ],
                "name": "FR",
                "priority": 500,
                "shortcut": 0
            }
        ]
    },
    {
        "enabled": 0,
        "hidden": 0,
        "hint": 0,
        "name": "Trans",
        "priority": 500,
        "shortcut": 0,
        "submenus": [
            {
                "enabled": 0,
                "hidden": 0,
                "hint": 0,
                "implementations": [
                    {
                        "noremap": 0,
                        "rhs": ":te trans :fr <cword>\r",
                        "sid": 0,
                        "silent": 0
                    }
                ],
                "name": "FR",
                "priority": 500,
                "shortcut": 0
            }
        ]
    },
    {
        "enabled": 0,
        "hidden": 0,
        "hint": 0,
        "name": "Search",
        "priority": 500,
        "shortcut": 0,
        "submenus": [
            {
                "enabled": 0,
                "hidden": 0,
                "hint": 0,
                "implementations": [
                    {
                        "noremap": 0,
                        "rhs": ":exe Grepper -grepprg rg --vimgrep $* $.",
                        "sid": 0,
                        "silent": 0
                    }
                ],
                "name": "CurrentBuffer",
                "priority": 500,
                "shortcut": 0
            },
            {
                "enabled": 0,
                "hidden": 0,
                "hint": 0,
                "implementations": [
                    {
                        "noremap": 0,
                        "rhs": ":exe Grepper -grepprg rg --vimgrep $* $+",
                        "sid": 0,
                        "silent": 0
                    }
                ],
                "name": "AllBuffers",
                "priority": 500,
                "shortcut": 0
            }
        ]
    },

It doesn't handle icons as neovim removed support for icons and it feels too much to have neovim handle icons IMO.

if that's ok I can proceed to clean up & write the tests ? any advice as how to write the advice ? I believe I can compare a lua table against the expored json ?

    local m = funcs.export_menus("","")
    m = funcs.json_encode(m)
@justinmk

This comment has been minimized.

Member

justinmk commented Apr 2, 2017

  • export_menus() doesn't seem the right name. Currently we have foo_encode(), foo_dump(), and foo_get(). Let's not add "export". I would think menu_get() is fine, as Bram mentioned in the Vim ticket.

  • If gVim has icon support for menus, we should restore that (at least as far as providing a field in the dictionary and :menu; how/if it is supported is decided by UIs). Not necessarily in this PR, but just FYI.

I believe I can compare a lua table against the expored json ?

If you get the map through a API function (e.g. nvim_get_menu()/nvim_buf_get_menu() pair, just like nvim_[buf_]get_keymap() in @tjdevries PR), it will automatically marshall to a Lua table which can be compared using helpers.eq().

@teto teto force-pushed the teto:menus branch from 869cea5 to 3f6a06f Apr 3, 2017

@teto

I will complete the tests when the form of the exported dict seems correct, maybe the mnemonic should be converted back to a chracter ?

@@ -5449,6 +5449,30 @@ max({list}) Return the maximum value of all items in {list}.
be used as a Number this results in an error.
An empty |List| results in zero.
menu_get({path}, {modes}) *dump_menu()*

This comment has been minimized.

@teto

teto Apr 3, 2017

Contributor

dump_menu => menu_get

ilent': 0, 'sid': 1, 'rhs': 'x'}, 'i': {'noremap': 1, 'enabled': 1, 'silent': 0, 'sid': 1, 'rhs': 'insert'},
'n': {'noremap': 1, 'enabled': 1, 'silent': 0, 'sid': 1, 'rhs': 'inormal'}}}], 'name': 'Test', 'priority':
500, 'shortcut': 84}
]

This comment has been minimized.

@teto

teto Apr 3, 2017

Contributor

ZyX-l suggested to add an example, too bad we can't generate it from the code. is there any vim-prettify that I could use to properly format this part ?

This comment has been minimized.

@justinmk

justinmk Apr 3, 2017

Member

Heh. Sadly gqq can't format Vim's own syntax. But I would:

  1. Join it to one line
  2. json_encode() it: :put =json_encode(<C-R>=getline('.')<CR>)
  3. Format it:
    :setl formatprg=python\ -m\ json.tool
    gqq
    

(If it's just showing a generic map, doesn't matter if it's JSON or VimL, I suppose. Though in this case the JSON is valid VimL)

@@ -85,6 +85,7 @@ return {
exists={args=1},
exp={args=1, func="float_op_wrapper", data="&exp"},
expand={args={1, 3}},
menu_get={args={1, 2}},

This comment has been minimized.

@teto

teto Apr 3, 2017

Contributor

Need to put in alphabetical order.

@teto teto force-pushed the teto:menus branch from 3e173f0 to b8757a6 Apr 3, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Apr 3, 2017

AFAIR it was not me who suggested to add an example. Actually documentation I write tends to be more “raw description” then something with examples. (I am not saying that this is how documentation should be written, only how I normally write it.)

As for how you get the results: use [helpers.]funcs.menu_get(args) for VimL functions like now. I am rather unsure about adding API function in the PR: calling VimL function is basically just as easy and it makes overhead (converting VimL value while you can just create Array directly, theoretically even create msgpack directly (need serious refactoring, see #4971)) more obvious.

@teto

This comment has been minimized.

Contributor

teto commented Apr 4, 2017

Maybe I misunderstood; from vim/vim#1563:

An example in the docs would be very helpful.

whatever, the example is helpful anyway. I don't think adding API functions is helpful either.

Without further comments, I consider the PR ready (functional tests pass locallly, hopes it passes here), icons & hints can be added later when UIs need it.

@teto teto changed the title from [WIP] Attempt at exporting menus to [RFC/RDY] Attempt at exporting menus Apr 4, 2017

@justinmk justinmk added this to the 0.2.1 milestone Apr 4, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Apr 4, 2017

@teto You are referring to vim/vim#1563 (comment)? This is comment from Bram, he is quoting me.

@teto teto force-pushed the teto:menus branch 2 times, most recently from 7d0c93a to 6d7f1df May 2, 2017

@teto teto force-pushed the teto:menus branch from 6d7f1df to 64ec478 Jun 16, 2017

@teto teto changed the title from [RFC/RDY] Attempt at exporting menus to [RDY] Attempt at exporting menus Jun 16, 2017

@marvim marvim added RDY and removed WIP labels Jun 18, 2017

@fmoralesc fmoralesc referenced this pull request Jul 1, 2017

Open

Descriptive Maps #6123

@teto teto force-pushed the teto:menus branch from 64ec478 to af09658 Jul 2, 2017

@teto teto force-pushed the teto:menus branch 2 times, most recently from 440e8ba to 670bebb Jul 19, 2017

teto added a commit to teto/nvim-palette that referenced this pull request Jul 24, 2017

Now supports menu items
You can try :PaletteMenu to just have menu items, :Palette should mix
both

neovim/neovim#6322
@teto

This comment has been minimized.

Contributor

teto commented Jul 24, 2017

I made a poc in this plugin (not optimized, still brittle but works locally) https://github.com/teto/nvim-palette. While waiting for #6288, I embed a copy https://github.com/teto/nvim-palette/tree/master/data in the plugin so the palette can load option descriptions.
Then if you use this PR with the plugin, you should have :PaletteMenu (only menu entries) working and :Palette should regroup both options and menu entries.

echo menu_get("")
>
should produce an output with a similar structure:
<

This comment has been minimized.

@justinmk

justinmk Jul 24, 2017

Member

I think these > ... < pairs are reversed.

    For instance, executing: >
     nnoremenu &Test.Test inormal
     inoremenu Test.Test insert
     vnoremenu Test.Test x
     echo menu_get("")
<
    should produce an output with a similar structure: >

This comment has been minimized.

@justinmk

justinmk Jul 24, 2017

Member

The sample output could be abbreviated like this:

{
    "hidden": 0,
    "name": "Test",
    "priority": 500,
    "shortcut": 84,
    "submenus": [
        {
            "hidden": 0,
            "implementations": {
                "i": {
                    "enabled": 1,
                    "noremap": 1,
                    "rhs": "insert",
                    "sid": 1,
                    "silent": 0
                },
                "n": { ... },
                "s": { ... },
                "v": { ... }
            },
            "name": "Test",
            "priority": 500,
            "shortcut": 0
        }
    ]
}

This comment has been minimized.

@teto

teto Jul 25, 2017

Contributor

several things were wrong with the doc ('>' pairs, spaces vs tabs, json format etc). I updated it and checked, I think it's fine now.

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 24, 2017

Instead of "implementations", "definitions" or "mappings" would be a more familiar name (if either of those makes sense in context).

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 24, 2017

I don't see any reason to keep this waiting. Anyone else have comments?

@teto teto force-pushed the teto:menus branch from 670bebb to 6e25bd4 Jul 25, 2017

@teto

This comment has been minimized.

Contributor

teto commented Jul 25, 2017

rebased, renamed "implementations" to "mappings".

Adds menu_get function to export menus to json
menu_get({path}, {modes}). See :h menu_get.

@teto teto force-pushed the teto:menus branch from 6e25bd4 to 5021625 Jul 25, 2017

@justinmk justinmk changed the title from [RDY] Attempt at exporting menus to [RDY] menu_get() / export menus Jul 25, 2017

@justinmk justinmk referenced this pull request Jul 27, 2017

Merged

rpc: close channel if stream was closed #7081

1 of 1 task complete
@justinmk

This comment has been minimized.

Member

justinmk commented Jul 28, 2017

merged. nicely done!

@justinmk justinmk closed this Jul 28, 2017

justinmk added a commit that referenced this pull request Jul 28, 2017

viml: introduce menu_get() function #6322
menu_get({path}, {modes}). See :h menu_get.

@teto teto deleted the teto:menus branch Jul 28, 2017

justinmk added a commit to justinmk/neovim that referenced this pull request Nov 8, 2017

NVIM v0.2.1
FEATURES:
0e873a3 Lua(Jit) built-in neovim#4411
5b32bce Windows: `:terminal` neovim#7007
7b0ceb3 UI/API: externalize cmdline neovim#7173
b67f58b UI/API: externalize wildmenu neovim#7454
b23aa1c UI: 'winhighlight' neovim#6597
17531ed UI: command-line coloring (`:help input()-highlight`) neovim#6364
244a1f9 API: execute lua directly from the remote api neovim#6704
45626de API: `get_keymap()` neovim#6236
db99982 API: `nvim_get_hl_by_name()`, `nvim_get_hl_by_id()` neovim#7082
dc68538 menu_get() function neovim#6322
9db42d4 :cquit : take an error code argument neovim#7336
9cc185d job-control: serverstart(): support ipv6 neovim#6680
1b7a9bf job-control: sockopen() neovim#6594
6efe84a clipboard: fallback to tmux clipboard neovim#6894
6016ac2 clipboard: customize clipboard with `g:clipboard` neovim#6030
3a86dd5 ruby: override ruby host via `g:ruby_host_prog` neovim#6841
16cce1a debug: $NVIM_LOG_FILE neovim#6827
0cba3da `:checkhealth` built-in, validates $VIMRUNTIME neovim#7399

FIXES:
105d680 TUI: more terminals, improve scroll/resize neovim#6816
cb912a3 :terminal : handle F1-F12, other keys neovim#7241
619838f inccommand: improve performance neovim#6949
04b3c32 inccommand: Fix matches for zero-width neovim#7487
60b1e8a inccommand: multiline, other fixes neovim#7315
f1f7f3b inccommand: Ignore leading modifiers in the command neovim#6967
1551f71 inccommand: fix 'gdefault' lockup neovim#7262
6338199 API: bufhl: support creating new groups neovim#7414
541dde3 API: allow K_EVENT during operator-pending
8c732f7 terminal: adjust for 'number' neovim#7440
5bec946 UI: preserve wildmenu during jobs/events neovim#7110
c349083 UI: disable 'lazyredraw' during ui_refresh. neovim#6259
51808a2 send FocusGained/FocusLost event instead of pseudokey neovim#7221
133f8bc shada: preserve unnamed register on restart neovim#4700
1b70a1d shada: avoid assertion on corrupt shada file neovim#6958
9f534f3 mksession: Restore tab-local working directory neovim#6859
de1084f fix buf_write() crash neovim#7140
7f76986 syntax: register 'Normal' highlight group neovim#6973
6e7a8c3 RPC: close channel if stream was closed neovim#7081
85f3084 clipboard: disallow recursion; show hint only once neovim#7203
8d1ccb6 clipboard: performance, avoid weird edge-cases neovim#7193
01487d4 'titleold' neovim#7358
01e53a5 Windows: better path-handling, separator (slash) hygiene neovim#7349
0f2873c Windows: multibyte startup arguments neovim#7060

CHANGES:
9ff0cc7 :terminal : start in normal-mode neovim#6808
032b088 lower priority of 'cursorcolumn', 'colorcolumn' neovim#7364
2a3bcd1 RPC: Don't delay notifications when request is pending neovim#6544
023f67c :terminal : Do not change 'number', 'relativenumber' neovim#6796
1ef2d76 socket.c: Disable Nagle's algorithm on TCP sockets neovim#6915
6720fe2 help: `K` tries Vim help instead of manpage neovim#3104
7068370 help, man.vim: change "outline" map to `gO` neovim#7405

@justinmk justinmk referenced this pull request Nov 8, 2017

Merged

NVIM v0.2.1 #7505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment