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

Improve search integration with the documentation panel #185

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

joguSD
Copy link
Contributor

@joguSD joguSD commented Sep 6, 2017

Previously, if you hit the input timeout while searching the documentation panel would close as input into the search buffer would be taken into consideration for the documentation to be shown. This PR does two things:

  1. Ensures that the documentation panel only considers the prompt's input for show documentation
  2. Will conditionally apply the search criteria against the prompt or documentation buffer based on which was previously focused before the search is activated

@joguSD
Copy link
Contributor Author

joguSD commented Sep 13, 2017

Trying to write tests for this by leveraging the internal interfaces will be quite challenging.
There's a vt100 terminal emulator called pyte that we could leverage to have the output of the shell go to an in memory string representation of the current terminal screen. Though, that only solves one problem and would likely also require a custom event loop that would allow us to provide input, inspect the current screen state, then provide more input, etc. I think it would be quite an effort to wire the prompt toolkit internals up in this way.

I also found hecate (by the same guy who wrote hypothesis) which is a testing framework designed for testing ncurses like terminal applications. It leverages tmux as a terminal emulator and interfaces with a tmux session to send input and determine the current screen state. It's relatively simple to write tests with it and I have one in this PR to test these changes. Unfortunately, actually running an instance of the shell to run tests against it currently is being blocked by having the documentation generate in the background. It would be better to have all the documentation already generated then run these kind of integration tests against them for consistency.

@joguSD
Copy link
Contributor Author

joguSD commented Sep 14, 2017

I got a proof of concept done for the pyte version. It currently side steps the event loop issue by spawning it in another thread while retaining a reference to the input stream and pyte screen. Might need to do some more interface shuffling on the AWSShell class to ensure all environment sensitive things can be injected rather than assumed (such as history).

This approach has some pretty significant advantages over hecate:

  1. No dependencies outside of python
  2. A lot faster
  3. Not as sensitive to timing issues
  4. We control the AWSShell instance being run.

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 this pull request may close these issues.

1 participant