[RFC] API: select items in popupmenu #9445
Conversation
760de05 to
df88efe
Compare
|
Update: there should be no reason to forbid this function to be called locally, so moved to |
4a42503 to
ea61f4a
Compare
|
Updated with docs and tests. |
src/nvim/edit.c
Outdated
| static bool pum_request_active = false; | ||
| static int pum_request_wanted_item = -1; | ||
| static bool pum_request_insert = false; | ||
| static bool pum_request_finish = false; |
There was a problem hiding this comment.
Does pum "request" imply RPC, or just "want"?
It might help navigation/discoverability if the "pum request" states were a flags enum:
enum {
kPumReqActive = 0x001,
kPumReqInsert = 0x002,
kPumReqDone = 0x004,
} PumRegState
static PumReqState pum_request = ...;There was a problem hiding this comment.
I guess want works. It is replacement for local variables for a function that cannot be written without either greatly restructuring the code, or perhaps refactor the compl data structures to something that can be easily manipulated without code duplication. Why erase the bool variables specifically?
There was a problem hiding this comment.
Why erase the bool variables specifically?
It's easier (for a human) to "track" one variable than many. It adds some implicit documentation, that all of these states are related.
There was a problem hiding this comment.
But wanted_item is precisely as related, it just happens to be of a different type. One could use a singleton struct struct { ... } pum_want; as a grouping I guess, and avoid unnecessary bit-twiddling.
src/nvim/edit.c
Outdated
| @@ -4342,6 +4364,16 @@ ins_compl_next ( | |||
| return num_matches; | |||
| } | |||
|
|
|||
| void pum_external_select_item(int item, bool insert, bool finish) { | |||
There was a problem hiding this comment.
pum_ext_select_item . We don't use "external" AFAICT. It's nice that "ext" means "extension" also, since "external" does not always make sense and we're more interested in extensibility.
N.B. I would propose renaming ui_is_external to ui_has, but waiting on the big PRs to merge first.
There was a problem hiding this comment.
N.B. I would propose renaming ui_is_external to ui_has, but waiting on the big PRs to merge first.
Fine by me. Just do it if you want it to happen, otherwise it will never happen. There will be outlying "big PRs" for quite some time, and it is not so hard to rebase fixup.
src/nvim/api/vim.c
Outdated
| /// @param finish If true, completion will be finished with this item, and the | ||
| /// popupmenu dissmissed. implies `insert`. | ||
| void nvim_select_popupmenu_item(Integer item, Boolean insert, Boolean finish, | ||
| Error *error) |
There was a problem hiding this comment.
should we bother to add a opts parameter for future extensions?
for mouse control and such.
Old code, haven't checked if it still works...working now.