Fork me on GitHub
#editors
<
2016-08-25
>
dominicm13:08:06

@juhoteperi: That bug you found in clj-async-omni seems to be an underlying bug in the nrepl-python-client

juhoteperi13:08:33

Hmm? Or just how nrepl-python-client works

juhoteperi13:08:58

Are you refering to the bug I fixed or some other bug?

dominicm13:08:04

With the original bdecode:

❯ python notadrill.py
>>  {'symbol': 'a.s.', 'ns': 'myns', 'op': 'complete'}
<< {}
<< session
<< c36ce69a-0a16-4323-af89-6d972dd6902c
<< status
<< ['done']
<< None
Swapping in the bdecode that fireplace uses:
>>  {'op': 'complete', 'symbol': 'a.s.', 'ns': 'teamhero.microformat'}
<< {'completions': [], 'status': ['done'], 'session': '92e73f6a-52b2-4c22-a9da-a3b274114a3d'}
--- The bug you fixed. I think it's good to do a truthy check there, but I think we were hitting it due to getting "None" back due to the bad bdecode

juhoteperi13:08:54

So looks like nrepl.bencode is calling connection read after it has already finished reading the message or something?

dominicm13:08:47

nrepl.bdecode seems to get confused and end the map prematurely (resulting in the {} on line 3)

dominicm13:08:14

That leaves additional data on the buffer, so it reads again.

dominicm13:08:07

Ah! and then we get None, because the real end of the map is left in the buffer, and nrepl.bdecode returns None if it doesn't understand

juhoteperi13:08:26

hmm, fireplace has working implementation written in python?

juhoteperi13:08:06

perhaps we could just copy nrepl_fireplace.py into clj-async-omni and modify it for our purposes

juhoteperi13:08:16

would also remove need for submodule

dominicm13:08:02

Yeah, that's my thoughts as well. Licensing and copyright slightly concerns me though, so I might have to dig in to that a little.

juhoteperi13:08:05

Would be easiest if you select to use Vim license for async-clj-omni, and then just add a comment to the file

juhoteperi13:08:35

But even if you select another license for the project, you can keep the copied file under Vim license

dominicm13:08:08

ah, misread your suggestion of using all of nrepl_fireplace.py.

dominicm13:08:57

There's a problem with that, nrepl_python_client provides a watchable connection, which is pretty key to being able to cache connections. So it would need implementing on top of nrepl_fireplace.

juhoteperi13:08:30

Hmm, true, fireplace implements that it Vimscript side

dominicm13:08:07

Does fireplace even use watchable connections? I thought the blocking nature of it, meant it could just keep reading for the duration of the evaluation.

juhoteperi13:08:35

Some part of fireplace code is async, only the Vim side blocks

dominicm13:08:21

Interesting.

dominicm13:08:53

Maybe I can figure out the bdecode bug, and fix it, using nrepl_fireplace as an example to draw inspiration from

dominicm14:08:59

Gotcha, it's a truthy check failure.

dominicm14:08:01

PR is opened.