Skip to content

Conversation

bfredl
Copy link
Member

@bfredl bfredl commented Sep 26, 2016

Fixes #5388, also I think some metadata attributes that are meant for internal use should not be exposed in nvim_get_api_info and api_info(). I don't think neither async nor receives_channel_id is interesting for an api user, but if someone is already using them, I could add them back.

@equalsraf
Copy link
Contributor

I'm unsure about the current role of async, at some point it was useful to force event handling by triggering another event after calling an async function.

@marvim marvim added the RFC label Sep 26, 2016
@bfredl
Copy link
Member Author

bfredl commented Sep 26, 2016

Note "async" here means executed directly in the low-level input loop, which currently is only nvim_input and nvim_get_api_info (but more static read-only methods could be, like nvim_get_color_map), it doesn't mean that the method can be called asynchronously, all functions can by being invoked async by using a msgpack-rpc notification.

@@ -174,7 +191,7 @@ static const uint8_t msgpack_metadata[] = {
]])
-- serialize the API metadata using msgpack and embed into the resulting
-- binary for easy querying by clients
packed = mpack.pack(functions)
packed = mpack.pack(exported_functions)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packed_exported_functions avoids confusion about the later code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 updated.

@bfredl
Copy link
Member Author

bfredl commented Sep 27, 2016

Also I'm not sure if can_fail should be exposed. It could be useful for clients to know that certain methods won't throw exceptions (all long as the client complies with the argument type requirements in the metadata), but then it could be a breaking change to change can_fail from false to true (by adding an Error* err out parameter to an API method.) While if exclude it, then auto-generated api clients must be prepared that all methods in principle could fail.

@justinmk
Copy link
Member

justinmk commented Sep 27, 2016

While if exclude it, then auto-generated api clients must be prepared that all methods in principle could fail.

I would lean towards excluding it for that reason. Though I wonder if we should take the "go" approach of always having a Error *err out-param, just to drive the point that no assumptions should be made. That also would remove the need for the can_fail attribute.

@bfredl
Copy link
Member Author

bfredl commented Sep 27, 2016

I agree, but in general I think we shouldn't say that the underlying C API is stable yet (for use of binary embedders and plugins), it is only stable as used through RPC as specified in the metadata, or via eval.

I also added a blanket implementation of "since". After 0.1.6 we of course will need to be able to specify this (and "deprecated_since") at method definitions.

exported_attributes = {'name', 'parameters', 'return_type', 'method',
'since', 'deprecated_since'}
exported_functions = {}
for i,f in ipairs(functions) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither i nor j are being used from these ipairs calls. The typical style is to use _ when the value is being discarded.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testlint enforces that. Would be nice to have lua-lint running on all of our lua logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 updated.

@bfredl bfredl merged commit 22dfe69 into neovim:master Sep 27, 2016
@jszakmeister jszakmeister removed the RFC label Sep 27, 2016
justinmk added a commit to justinmk/neovim that referenced this pull request Oct 27, 2016
FEATURES:
0b5a7e4 neovim#4432 API: external UIs can render custom popupmenu
c6ac4f8 neovim#4934 API: call any API method from vimscript
31df051 neovim#4568 API: nvim_call_atomic(): multiple calls in a single request
b268ba3 neovim#5424 API: nvim_win_get_number(), nvim_tabpage_get_number()
e7e2844 has("nvim-1.2.3") checks for a specific Nvim version
522b885 neovim#5295, neovim#5493 `:CheckHealth` checks tmux, terminfo, performance
719dae2 neovim#5384 events: allow event processing in getchar()
f25797f neovim#5386 API: metadata: Nvim version & API level
22dfe69 neovim#5389 API: metadata: "since", "deprecated_since"
605e743 Added QuickFixLine highlight group

CHANGES:
4af6ec7 neovim#5253 perf: Disable clipboard in do_cmdline()
6e9f329 neovim#5299 perf: Skip foldUpdate() in insert-mode.
9d4fcec neovim#5426 perf: Do not auto-update folds for some foldmethods.
eeec0ca neovim#5419 tui: Default to normal-mode cursor shape.

FIXES:
e838452 neovim#5436 tui: Fix "weird characters" / "bleeding termcodes"
10a54ad neovim#5243 signal_init: Always unblock SIGCHLD.
bccb49b neovim#5316 eval.c: Fix memory leak for detached pty job
626065d neovim#5227 tchdir: New tab should inherit CWD.
cd321b7 neovim#5292 getcwd(): Return empty string if CWD is invalid.
6127eae shada: Fix non-writeable ShaDa directory handling
ca65514 neovim#2789 system(): Respect shellxescape, shellxquote
2daf54e neovim#4874 Restore vim-like tab dragging
0c536b5 neovim#5319 syntax.c: Support bg/fg special color-names.
3c53371 neovim#4972 from justinmk/schedule-ui_refresh
68bcb32 neovim#4789 tui.c: Do not wait for tui loop on teardown.
c8b6ec2 neovim#5409 v:count broken in command-line window
6bc3bce neovim#5461 fix emoji display
51937e1 neovim#5470 fix :terminal with :argadd, :argu
79d77da neovim#5481 external UIs: opening multiple files from command-line
657ba62 neovim#5501 rplugin: resolve paths in manifest file
6a6f188 neovim#5502 system('foo &', 'bar'): Show error, don't crash.
1ff162c neovim#5515 os_nodetype: open fd with O_NONBLOCK
2a6c5bb neovim#5450 modeline: Handle version number overflow.
0ade1bb neovim#5225 CI tests now run against Windows!
justinmk added a commit to justinmk/neovim that referenced this pull request Oct 28, 2016
FEATURES:
0b5a7e4 neovim#4432 API: external UIs can render custom popupmenu
c6ac4f8 neovim#4934 API: call any API method from vimscript
31df051 neovim#4568 API: nvim_call_atomic(): multiple calls in a single request
b268ba3 neovim#5424 API: nvim_win_get_number(), nvim_tabpage_get_number()
e7e2844 has("nvim-1.2.3") checks for a specific Nvim version
522b885 neovim#5295, neovim#5493 `:CheckHealth` checks tmux, terminfo, performance
719dae2 neovim#5384 events: allow event processing in getchar()
f25797f neovim#5386 API: metadata: Nvim version & API level
22dfe69 neovim#5389 API: metadata: "since", "deprecated_since"
605e743 Added QuickFixLine highlight group

CHANGES:
4af6ec7 neovim#5253 perf: Disable clipboard in do_cmdline()
6e9f329 neovim#5299 perf: Skip foldUpdate() in insert-mode.
9d4fcec neovim#5426 perf: Do not auto-update folds for some foldmethods.
eeec0ca neovim#5419 tui: Default to normal-mode cursor shape.

FIXES:
e838452 neovim#5436 tui: Fix "weird characters" / "bleeding termcodes"
10a54ad neovim#5243 signal_init: Always unblock SIGCHLD.
bccb49b neovim#5316 eval.c: Fix memory leak for detached pty job
626065d neovim#5227 tchdir: New tab should inherit CWD.
cd321b7 neovim#5292 getcwd(): Return empty string if CWD is invalid.
6127eae shada: Fix non-writeable ShaDa directory handling
ca65514 neovim#2789 system(): Respect shellxescape, shellxquote
2daf54e neovim#4874 Restore vim-like tab dragging
0c536b5 neovim#5319 syntax.c: Support bg/fg special color-names.
3c53371 neovim#4972 from justinmk/schedule-ui_refresh
68bcb32 neovim#4789 tui.c: Do not wait for tui loop on teardown.
c8b6ec2 neovim#5409 v:count broken in command-line window
6bc3bce neovim#5461 fix emoji display
51937e1 neovim#5470 fix :terminal with :argadd, :argu
79d77da neovim#5481 external UIs: opening multiple files from command-line
657ba62 neovim#5501 rplugin: resolve paths in manifest file
6a6f188 neovim#5502 system('foo &', 'bar'): Show error, don't crash.
1ff162c neovim#5515 os_nodetype: open fd with O_NONBLOCK
2a6c5bb neovim#5450 modeline: Handle version number overflow.
0ade1bb neovim#5225 CI tests now run against Windows!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants