Skip to content

Implements neovim/neovim@9d77a07 APIs #49

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

Merged
merged 3 commits into from
May 11, 2019
Merged

Implements neovim/neovim@9d77a07 APIs #49

merged 3 commits into from
May 11, 2019

Conversation

zchee
Copy link
Member

@zchee zchee commented May 10, 2019

Implements neovim/neovim@9d77a07 APIs.

compare is here:

> nvim_buf_clear_namespace(buffer Buffer, ns_id Integer, line_start Integer, line_end Integer) void { name(nvim_buf_clear_namespace) }
> nvim_buf_get_offset(buffer Buffer, index Integer) Integer { name(nvim_buf_get_offset) }
> nvim_buf_set_virtual_text(buffer Buffer, ns_id Integer, line Integer, chunks Array, opts Dictionary) Integer { name(nvim_buf_set_virtual_text) }
> nvim_create_buf(listed Boolean, scratch Boolean) Buffer { name(nvim_create_buf) }
> nvim_create_namespace(name String) Integer { name(nvim_create_namespace) }
> nvim_get_namespaces() Dictionary { name(nvim_get_namespaces) }
> nvim_input_mouse(button String, action String, modifier String, grid Integer, row Integer, col Integer) void { name(nvim_input_mouse) }
> nvim_open_win(buffer Buffer, enter Boolean, config Dictionary) Window { name(nvim_open_win) }
> nvim_select_popupmenu_item(item Integer, insert Boolean, finish Boolean, opts Dictionary) void { name(nvim_select_popupmenu_item) }
> nvim_set_vvar(name String, value Object) void { name(nvim_set_vvar) }
> nvim_ui_try_resize_grid(grid Integer, width Integer, height Integer) void { name(nvim_ui_try_resize_grid) }
> nvim_win_close(window Window, force Boolean) void { name(nvim_win_close) }
> nvim_win_get_config(window Window) Dictionary { name(nvim_win_get_config) }
> nvim_win_set_buf(window Window, buffer Buffer) void { name(nvim_win_set_buf) }
> nvim_win_set_config(window Window, config Dictionary) void { name(nvim_win_set_config) }
----
< nvim_set_client_info(name String, version Dictionary, typ String, methods map[string]*ClientMethod, attributes Dictionary) void { name(nvim_set_client_info) }
> nvim_set_client_info(name String, version Dictionary, type String, methods Dictionary, attributes Dictionary) void { name(nvim_set_client_info) }

Note that nvim_set_client_info diff is only of typ arg, type is Go reserve word.


If you find a doc that is not appropriate, please give a line comment. (Note that I know different to the latest nvim doc/*.txt document comment.)
I will fix this PR, Or there are also inappropriate comments in other places, so I will fix it in the next PR.

@zchee zchee requested a review from justinmk May 10, 2019 17:04
@zchee zchee self-assigned this May 10, 2019
@justinmk
Copy link
Member

If you find a doc that is not appropriate, please give a line comment. (Note that I know different to the latest nvim doc/*.txt document comment.)

I would just wait until it can be automated as discussed.

@justinmk
Copy link
Member

Note that nvim_set_client_info diff is only of typ arg, type is Go reserve word.

what about type_ instead?

@zchee
Copy link
Member Author

zchee commented May 10, 2019

what about type_ instead?

it's Go idiom. basically, Go doesn't use underscore for variable, package name.
See https://github.com/golang/tools/blob/a434f64ace81347eff0fb4a32bc80a235e0ad762/go/pointer/query.go#L36

@zchee
Copy link
Member Author

zchee commented May 11, 2019

@justinmk

I would just wait until it can be automated as discussed.

Yes, I mean Please point out any obvious mistakes if there. such as "a Nvim" or etc.

@zchee
Copy link
Member Author

zchee commented May 11, 2019

for note:

it's Go idiom. basically, Go doesn't use underscore for variable, package name.

expect .go filename.

@justinmk
Copy link
Member

LGTM. which parts of this are auto-generated?

Is the auto-generator process documented anywhere? One or two lines with a usage example in the README.md would be great.

@zchee
Copy link
Member Author

zchee commented May 11, 2019

@justinmk No! always hand writing!! lol
saw C source, doc txt or etc...

@justinmk
Copy link
Member

justinmk commented May 11, 2019

apiimp is supposed to be auto-generated using apitool.

// Command apitool generates apiimp.go from apidef.go. The command also has

see also:

// Code generated by running "go generate" in github.com/neovim/go-client/nvim. DO NOT EDIT.

can you try that, and push the result here?

@zchee
Copy link
Member Author

zchee commented May 11, 2019

Ah, but write API boilerplate to apidef.go, and run go generate. then updated apiimp.go. It's development flow.
If you said means doc comment, arg or etc, those are almost hand written.

you said which meaning?

@justinmk
Copy link
Member

So, the version of apiimp.go that you have posted here, is auto-generated?

@zchee
Copy link
Member Author

zchee commented May 11, 2019

Ah, I see. you said means developer flow?

If so, I'll create CONTRIBUTING.md or write flow to readme.

@justinmk
Copy link
Member

Just README.md is enough (1 or 2 lines, plus example).

@zchee
Copy link
Member Author

zchee commented May 11, 2019

So, the version of apiimp.go that you have posted here, is auto-generated?

Yes, correct. apiimp.go generated based by apidef.go.

@zchee
Copy link
Member Author

zchee commented May 11, 2019

Just README.md is enough (1 or 2 lines, plus example).

I see, will do.
But I'm not native English speaker, pls pointed out sentence or etc when I send PR.

@zchee
Copy link
Member Author

zchee commented May 11, 2019

@justinmk
Also, I have a suggestion. go-client has been implemented all now latest Neovim APIs when merge this PR.
I said before, Go developer(and Go core team) suggest use semver.
So, I think it's good timing to release 1.0.0(but lack one file, go.mod. will PR. See https://github.com/golang/go/wiki/Modules#quick-start), what do you think?

@justinmk justinmk merged commit eaaca6f into master May 11, 2019
@justinmk justinmk deleted the api/9d77a07 branch May 11, 2019 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants