Initialize the agent #34
Conversation
|
Will wait on @jamiboy16 to review #29 so I can merge it into this branch - will review this after. That way you can pull + test it with the |
|
Sounds good 👍 |
* Created initial vim plugin (python-supported versions of vim only) * Use test client code in vim plugin. Spawn and communicate with agent instance * Add python3 check
| """ | ||
| def __init__(self, contents): | ||
| self.type = EditorProtocolMessageType.ApplyText | ||
| self.contents = contents |
There was a problem hiding this comment.
I am changing this to enforce that contents is an array of strings (each item represents a line in the buffer). Makes it easier to work with plugin APIs. Doing it in a future diff, just a heads up.
agent/main.py
Outdated
| def set_up_logging(): | ||
| logging.basicConfig( | ||
| level=logging.DEBUG, | ||
| format='%(asctime)s %(levelname)-8s %(message)s', |
There was a problem hiding this comment.
We switch b/w single quotes and double quotes for strings - should we agree on one and just keep it consistent? Doesn't really matter IMO but just saying.
There was a problem hiding this comment.
Yeah I'm fine with either. I think this was a result of copypasta from a code example. What is the convention for Python?
There was a problem hiding this comment.
If we go by PEP 8, it doesn't matter as long as you stick with one and keep it consistent. Since we use double quotes in most places, and I used them as well, let's stick with double.
There was a problem hiding this comment.
Ok, I'll change it over. I'll run pep8 over this too.
|
|
||
|
|
||
| def signal_handler(signal, frame): | ||
| global should_shutdown |
There was a problem hiding this comment.
Why do we use global here? Wouldn't it be already declared in line 6?
There was a problem hiding this comment.
I thought that originally too, but it seems like you need to explicitly declare that you're referencing a global variable in Python (I think when making some mutating call). It didn't work for me without the global declaration.
agent/tandem/io/std_streams.py
Outdated
| class StdStreams: | ||
| def __init__(self, handler_function): | ||
| self._handler_function = handler_function | ||
| self._reader = Thread(target=self._stdin_read) |
There was a problem hiding this comment.
I think as discussed, to avoid having to explicitly put Do not call directly on _stdin_read, we can instead make a function that returns a new Thread with the _stdin_read encapsulated within it.
There was a problem hiding this comment.
Yeah that's a good idea. I moved it into a _get_read_thread() function that declares _stdin_read as a nested function.
| @@ -0,0 +1,77 @@ | |||
| import json | |||
There was a problem hiding this comment.
I think we're going to want to split this into multiple files. In the future, it should be okay like this for now.
There was a problem hiding this comment.
Yeah that makes sense. Since this defines messages between the agent and editor plugin, we may even want to consider using something like Protobuf/Thrift to define the messages so that they can easily be generated for different languages.
|
I'll merge this as-is and rebase my socket changes to prevent this from growing too large. |
This initializes the agent adding the ability for it to communicate with its parent process through
stdinandstdout. The code is in Python 3. The logs are written to/tmp/tandem-agent.logsince we can't usestdinnorstdoutbecause they are used for IPC. The plan is to make this location configurable in the future.It turns out that I should actually be adding blank
__init__.pyfiles in the various packages. I avoided adding them in now to not conflict with the changes in #29.Messages received from the editor plugin are processed in
agent/tandem/protocol/editor/handler.py. Communication with the plugin is done usingagent/tandem/io/std_streams.py. All the business logic in the agent should be running in the main executor to ensure that we're running on a single thread.