Skip to content
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

8 bytes #29

Closed
Sineaggi opened this issue Apr 29, 2016 · 3 comments · Fixed by #50
Closed

8 bytes #29

Sineaggi opened this issue Apr 29, 2016 · 3 comments · Fixed by #50

Comments

@Sineaggi
Copy link

Why do we send the 8 bytes back and forth when dealing with stdio? We certainly don't do that when dealing with with stderr.

I understand the 8 bytes are to tell the client/server how much data we're sending over, but if we're using json and prepending a custom binary sequence to the beginning of the transmission, why not go full binary for the interface? Or the opposite question, why not go full json? We already push an extra '\n' at the end.

@raphlinus
Copy link
Member

It's a somewhat arbitrary decision, and possibly subject to change. If you know the length, the code to deal with it is simple and efficient, and it doesn't place restrictions on the content. There is one message format planned for the future where I suspect JSON overhead will be a problem, which is measuring the width of substrings (an RPC from the core to the front-end). If I do that, then having binary framing will be essential. Doing binary-everything would either (a) make the code a lot harder to read, write and maintain, or (b) require some kind of binary formatting like protobuf, which would make the build system harder.

For plugins, I probably will go full JSON, delimited by \n, as it's the ultimate in programmer friendliness.

@Sineaggi
Copy link
Author

Sineaggi commented Apr 29, 2016

Ok, that's fair.

One other question: I've been working on a Windows UI based on WPF, and every so often, along with the first 8 bytes, I get 7 extra bytes, with values [1,0,0,0,0,0,0], then the JSON string. Do you have any idea why this happens? I've narrowed it down to when the first byte sent over the wire is 10. I'm not sure why, but it happens ever multiple of 256. Also, it only happens when I'm appending to a single line. :) Not sure why.

The first time it occurs is when the bytes sent reached 266. That's a 10 in the first spot, then a 1 in the second byte's spot.

EDIT: After a bit of investigation, the stream I'm getting contains 10,a,b,c,d,e,f,g,a,b,c,d,e,f,g[rest of json here]. So the 7 bytes after the first 10 byte is being duplicated somewhere. I have to consume those before the rest of the JSON message can be read. Wonder if this is a Windows thing.

@raphlinus
Copy link
Member

@Sineaggi That sure sounds like it's doing LF to CRLF conversion, ie not treating the channel as binary-clean.

raphlinus added a commit that referenced this issue May 1, 2016
A significant restructuring, motivated to allow editing of more than
one tab at a time. With this commit, the front-end/back-end protocol
supports multiple tabs, but that functionality is not implemented in
the front-end yet.

The RPC mechanism has been reworked to be much more like JSON-RPC 2.
Thus, this patch closes #29.

This also closes #44, which describes the motivation for these
changes.
lord pushed a commit to lord/xi-editor that referenced this issue Oct 31, 2018
A significant restructuring, motivated to allow editing of more than
one tab at a time. With this commit, the front-end/back-end protocol
supports multiple tabs, but that functionality is not implemented in
the front-end yet.

The RPC mechanism has been reworked to be much more like JSON-RPC 2.
Thus, this patch closes xi-editor#29.

This also closes xi-editor#44, which describes the motivation for these
changes.
lord pushed a commit to lord/xi-editor that referenced this issue Oct 31, 2018
Add "gesture" for Command-click
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 a pull request may close this issue.

2 participants