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

Autocomplete #2023

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from

Conversation

nullvariable
Copy link
Contributor

Adds a feature for #1595

Provides an autocomplete feature compatible with zsh and bash-completion. Allows for easy tab completion in supported shells.

I may have over complicated this, but in order to minimize the distribution size I've adopted a strategy for caching the autocomplete script. This allows us to only generate the autocomplete script when building the phar file, since we don't need the script to be dynamic. I suppose one downside of this is that it may not pickup plugins.

Very open to additional thoughts and improvements. Currently requires that you set the following in your .zshrc file:

eval "$(terminus autocomplete)"

terminus-autocomplete

Copy link
Contributor

@TeslaDethray TeslaDethray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to get this to work - when I eval the autocomplete output I get an unexpected EOF error.

$local_machine_helper = $this->getContainer()->get(LocalMachineHelper::class);
if (!$local_machine_helper->getFilesystem()->exists($filename)) {
throw new TerminusNotFoundException(
'Please generate the autocomplete script using the `composer autocomplete:build` command.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a different message if you are running Terminus via a PHAR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory the Phar should have the compiled/cached autocomplete script that's generated during the build process, this is for someone running the composer version

*/
protected function getFilename()
{
return $this->config->get('assets_dir') . "/autocomplete.txt";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go into the cache dir. The assets dir holds the ASCII art.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll adjust the location

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TeslaDethray so my intent with this was to cache the autocomplete script so it could be included with the Phar instead of including all the libraries used to generate it. The cache dir looks like it's user specific, and wouldn't be bundled into the phar. Would it make sense to create a separate folder just for caching the autocomplete script?

$autocomplete = shell_exec('symfony-autocomplete bin/terminus');
file_put_contents('assets/autocomplete.txt', $autocomplete);
} else {
echo "Please install dev dependencies, or run composer global require bamarni/symfony-console-autocomplete";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project in question recommends composer global require, but we should not.

https://pantheon.io/blog/fixing-composer-global-command

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, never mind. bamarni/symfony-console-autocomplete has no dependencies, so it's actually safe to use it with composer global require.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, my assumption is that almost no one will ever run this, most people will get this via the CI generated Phar file. This will only be needed using Terminus via composer

@dwasyluk
Copy link

dwasyluk commented Nov 30, 2019

Would love to see this merged. In the meantime @nullvariable or @stevector can you educate my noob self on how the composer solution works? Do I need to install composer on all the machines i want autocomplete to work with or just on the client i am SSH'ing from? This seems like a pretty big oversight on the termius devs part. Thanks for any help! TAB autocomplete === 👼

@nullvariable
Copy link
Contributor Author

@dwasyluk composer is only needed if you're building terminus from source. If you're using the packaged phar version, composer isn't needed.

Right now to use this branch you would have to install composer and generate the phar file yourself, but that phar is portable and you could use on any machine with php. Once this is completed and merged the build process will generate all of this for you.

@uberhacker
Copy link
Contributor

Hey @nullvariable: I just now noticed this PR. Nice work! If it helps any, I have created an autocomplete terminus plugin. See https://github.com/terminus-plugin-project/terminus-autocomplete-plugin. It would be cool if we could collaborate and combine our ideas.

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.

None yet

5 participants