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

[RFC] Implement basic --embedded-mode command-line option #1060

Merged
merged 7 commits into from Aug 28, 2014

Conversation

Projects
None yet
6 participants
@tarruda
Copy link
Member

tarruda commented Aug 12, 2014

This option provides a simple way to embed Neovim into another process by starting an msgpack-rpc channel via stdin/stdout. This will be used run tests against a headless nvim instance and to help adapting the old terminal code to msgpack-rpc. Though the headless mode requires #781 to be "complete", I decided to make this a separate pull request.

@tarruda tarruda force-pushed the tarruda:embedded-mode branch 3 times, most recently from a4d622e to 68e14d8 Aug 21, 2014

test: Remove cleanup function definition from run-api-tests.exp
This function is now injected automatically when running the python-client tests

@tarruda tarruda force-pushed the tarruda:embedded-mode branch from c955ffe to eac3299 Aug 28, 2014

tarruda added some commits Aug 12, 2014

wstream: Implement wstream_set_file
It's analogous to rstream_set_file but only supports pipes(Support for regular
files may be added later). This function was added to support creating API
channels via stdout.
channel: Implement channel_from_stdio function
This function can be used to create an API channel that reads/writes from/to
stdin/stdout
api: Implement '--embedded-mode' command-line option
This option makes nvim run in "embedded mode", which creates an API channel via
stdin/stdout and disables all terminal-related code

@tarruda tarruda force-pushed the tarruda:embedded-mode branch from eac3299 to 3dfa2b2 Aug 28, 2014

@tarruda tarruda force-pushed the tarruda:embedded-mode branch from 3dfa2b2 to dd90dbe Aug 28, 2014

@tarruda tarruda merged commit dd90dbe into neovim:master Aug 28, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

tarruda added a commit that referenced this pull request Aug 28, 2014

@tarruda tarruda referenced this pull request Sep 7, 2014

Merged

September Newsletter #71

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Sep 9, 2014

I probably only glanced over this when it came to pass, but if I wouldn't have, I would've made one remark:

Why not call it --embedded instead of --embedded-mode? I know it seems like bike-shedding, but it doesn't seem to lose a lot of information and it's easier to remember. At least we're still in a position of being able to make breaking changes before release (unless @myitcv gets unreasonably angry, that is) ;).

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Sep 9, 2014

@aktau I agree, I kind of assumed we would tighten up the option names nearing a release. The messagepack-api option is also very long and breaks the --help layout.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Sep 9, 2014

Why not call it --embedded instead of --embedded-mode

👍 I will squeeze a commit in #1130

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Sep 9, 2014

@justinmk / @aktau What about --embed instead?

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Sep 9, 2014

@tarruda 👍

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Sep 9, 2014

messagepack-api option is also very long and breaks the --help layout.

Do you have an alternative for --api-msgpack-metadata ? I was thinking of simply cutting the "msgpack" to --api-metadata

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Sep 9, 2014

@tarruda That sounds good, or maybe even just --api. I could see the reasoning behind the explicit naming, but command-line options have relatively limited purpose, so the risk of naming conflicts and/or ambiguity is low.

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Sep 9, 2014

@justinmk / @aktau What about --embed instead?

👍

Do you have an alternative for --api-msgpack-metadata ? I was thinking of simply cutting the "msgpack" to --api-metadata

Perhaps --msgpack-api or even --api as @justinmk said.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Sep 9, 2014

Perhaps --msgpack-api or even --api as @justinmk said.

It is now --api-info: tarruda@7c90700

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Sep 9, 2014

Great!

@myitcv

This comment has been minimized.

Copy link
Member

myitcv commented Sep 9, 2014

Sounds good to me!
On 9 Sep 2014 15:29, "Nicolas Hillegeer" notifications@github.com wrote:

I probably only glanced over this when it came to pass, but if I wouldn't
have, I would've made one remark:

Why not call it --embedded instead of --embedded-mode? I know it seems
like bike-shedding, but it doesn't seem to lose a lot of information and
it's easier to remember. At least we're still in a position of being able
to make breaking changes before release (unless @myitcv
https://github.com/myitcv gets unreasonably angry, that is) ;).


Reply to this email directly or view it on GitHub
#1060 (comment).

@trusktr

This comment has been minimized.

Copy link

trusktr commented Nov 16, 2014

Why not call it --embedded instead of --embedded-mode

Or --headless? :P

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