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

New 'completions' command, and various tab-completion fixes and improvements #649

Open
wants to merge 45 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
9b33ce3
Document the word-break limitations of the Readline completion
Aug 9, 2020
a9f5916
Add 'completions' command
Jul 23, 2020
05afb93
Add 'completions' to the needCompleteClass() list
Aug 9, 2020
d177094
Make AbstractMatcher::startsWith() treat $prefix as a verbatim string
Jul 23, 2020
025784a
Add tokenIsValidIdentifier() for generic tab-completion tests
Jul 30, 2020
6b8a9d1
Use tokenIsValidIdentifier() in getInput() instead of matching T_STRING
Jul 23, 2020
69739d1
Renaming variables for consistency
Jul 30, 2020
4bb2898
Do not return completions if valid input was not obtained
Jul 30, 2020
f40b749
Add missing documentation
Jul 30, 2020
20a5c0a
Bug fix for ClassNamesMatcher::hasMatched()
Jul 30, 2020
3c92986
Bug fix for KeywordsMatcher::hasMatched()
Jul 30, 2020
17d1c4b
Bug fix for CommandsMatcher::hasMatched()
Jul 30, 2020
e0c0b2a
Use tokenIsValidIdentifier() instead of T_STRING matching in hasMatch…
Jul 30, 2020
f206480
Use tokenIsValidIdentifier() with T_OBJECT_OPERATOR matching in hasMa…
Jul 30, 2020
3cd36d0
Comments
Jul 30, 2020
63ae75d
Minor refactoring
Aug 9, 2020
f12e1e8
If not completing a valid prefix, consider it an empty string
Jul 30, 2020
21b1f38
Don't trim trailing spaces and tabs from command input
Jul 29, 2020
1655584
Treat T_OPEN_TAG and ';' the same way for the purposes of completion
Aug 14, 2020
0611ead
Support string tokens in AbstractMatcher::hasToken() and tokenIs()
Aug 7, 2020
3a829c7
Fix inconsistencies with completion of variables
Aug 14, 2020
2b566fe
Improve the 'previous token' blacklisting in hasMatched() methods
Aug 7, 2020
c66e4aa
Bug fix for getNamespaceAndClass()
Aug 9, 2020
4c7a285
Allow an incomplete class name in getNamespaceAndClass()
Aug 9, 2020
80717d0
fixup! Add 'completions' command
Aug 16, 2020
72a0924
fixup! Add tokenIsValidIdentifier() for generic tab-completion tests
Aug 17, 2020
4ae9f12
fixup! Use tokenIsValidIdentifier() in getInput() instead of matching…
Aug 16, 2020
bfaeb46
fixup! Use tokenIsValidIdentifier() instead of T_STRING matching in h…
Aug 17, 2020
61fd4d2
fixup! If not completing a valid prefix, consider it an empty string
Aug 16, 2020
09e7422
fixup! Support string tokens in AbstractMatcher::hasToken() and token…
Aug 16, 2020
91c485f
fixup! Fix inconsistencies with completion of variables
Aug 16, 2020
e585663
fixup! Allow an incomplete class name in getNamespaceAndClass()
Aug 17, 2020
4d0041b
fixup! Fix inconsistencies with completion of variables
Aug 17, 2020
a16f51c
fixup! Add 'completions' command
Aug 17, 2020
0e0b3a1
fixup! If not completing a valid prefix, consider it an empty string
Aug 17, 2020
454e3db
fixup! Use tokenIsValidIdentifier() in getInput() instead of matching…
Aug 17, 2020
f00e8c2
fixup! Add tokenIsValidIdentifier() for generic tab-completion tests
Aug 17, 2020
cdd69c2
fixup! Add tokenIsValidIdentifier() for generic tab-completion tests
Aug 17, 2020
5a22d52
fixup! Fix inconsistencies with completion of variables
Aug 17, 2020
51f66e1
fixup! Document the word-break limitations of the Readline completion
Aug 17, 2020
cf8a487
fixup! Add 'completions' command
Aug 17, 2020
57be706
fixup! Support string tokens in AbstractMatcher::hasToken() and token…
Aug 17, 2020
bb27322
fixup! Add tokenIsValidIdentifier() for generic tab-completion tests
Aug 17, 2020
b89af21
Update the list of commands in AbstractMatcher::needCompleteClass()
Aug 17, 2020
695f0d5
fixup! Update the list of commands in AbstractMatcher::needCompleteCl…
Aug 17, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions src/Command/CompletionsCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

/*
* This file is part of Psy Shell.
*
* (c) 2012-2020 Justin Hileman
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Psy\Command;

use Psy\Input\CodeArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

/**
* Dump an array of possible completions for the given input.
*/
class CompletionsCommand extends Command
{
/**
* {@inheritdoc}
*/
protected function configure()
{
$this
->setName('completions')
->setDefinition([
new CodeArgument('target', CodeArgument::OPTIONAL, 'PHP code to complete.'),
])
->setDescription('List possible code completions for the input.')
->setHelp(
<<<'HELP'
This command enables PsySH wrappers to obtain completions for the current
input, for the purpose of implementing their own completion UI.
HELP
);
}

/**
* {@inheritdoc}
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
$target = $input->getArgument('target');
if (!isset($target)) {
$target = '';
}

// n.b. All of the relevant parts of \Psy\Shell are protected
// or private, so getTabCompletions() itself is a Shell method.
$completions = $this->getApplication()->getTabCompletions($target);

// Ouput the completion candidates as newline-separated text.
$str = implode("\n", array_filter($completions))."\n";
$output->write($str, false, OutputInterface::OUTPUT_RAW);

return 0;
}
}
5 changes: 5 additions & 0 deletions src/Input/CodeArgument.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
* parse function() { return "wheee\n"; }
*
* ... without having to put the code in a quoted string and escape everything.
*
* Certain trailing whitespace characters are exceptions. Trailing Spaces and
* tabs will be included in the argument value, but trailing newlines, carriage
* returns, vertical tabs, and nulls are trimmed from the command before the
* arguments are established.
*/
class CodeArgument extends InputArgument
{
Expand Down
19 changes: 18 additions & 1 deletion src/Shell.php
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,23 @@ protected function getTabCompletionMatchers()
@\trigger_error('getTabCompletionMatchers is no longer used', \E_USER_DEPRECATED);
}

/**
* Get completion matches.
*
* @return array An array of completion matches for $input
*/
public function getTabCompletions(string $input)
{
$ac = $this->autoCompleter;
$word = '';
$regexp = $ac::WORD_REGEXP;
phil-s marked this conversation as resolved.
Show resolved Hide resolved
$matches = [];
if (preg_match($regexp, $input, $matches) === 1) {
$word = $matches[0];
}
return $ac->processCallback($word, null, ['line_buffer' => $input]);
}

/**
* Gets the default command loop listeners.
*
Expand Down Expand Up @@ -909,7 +926,7 @@ protected function runCommand($input)
throw new \InvalidArgumentException('Command not found: '.$input);
}

$input = new ShellInput(\str_replace('\\', '\\\\', \rtrim($input, " \t\n\r\0\x0B;")));
$input = new ShellInput(\str_replace('\\', '\\\\', \rtrim($input, "\n\r\0\x0B;")));

if ($input->hasParameterOption(['--help', '-h'])) {
$helpCommand = $this->get('help');
Expand Down
121 changes: 119 additions & 2 deletions src/TabCompletion/AutoCompleter.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,84 @@ class AutoCompleter
/** @var Matcher\AbstractMatcher[] */
protected $matchers;

/**
* The set of characters which separate completeable 'words', and
* therefore determines the precise $input word for which completion
* candidates should be generated (noting that each candidate must
* begin with the original $input text).
phil-s marked this conversation as resolved.
Show resolved Hide resolved
*
* PHP's readline support does not provide any control over the
* characters which constitute a word break for completion purposes,
* which means that we are restricted to the default -- being the
* value of GNU Readline's rl_basic_word_break_characters variable:
*
* The basic list of characters that signal a break between words
* for the completer routine. The default value of this variable
* is the characters which break words for completion in Bash:
* " \t\n\"\\’‘@$><=;|&{(".
*
* This limitation has several ramifications for PHP completion:
*
* 1. The namespace separator '\' introduces a word break, and so
* class name completion is on a per-namespace-component basis.
* When completing a namespaced class, the (incomplete) $input
* parameter (and hence the completion candidates we return) will
* not include the separator or preceding namespace components.
*
* 2. The double-colon (nekudotayim) operator '::' does NOT introduce
* a word break (as ':' is not a word break character), and so the
* $input parameter will include the preceding 'ClassName::' text
* (typically back to, but not including, a space character or
* namespace-separator '\'). Completion candidates for class
* attributes and methods must therefore include this same prefix.
*
* 3. The object operator '->' introduces a word break (as '>' is a
* word break character), so when completing an object attribute
* or method, $input will contain only the text following the
* operator, and therefore (unlike '::') the completion candidates
* we return must NOT include the preceding object and operator.
*
* 4. '$' is a word break character, and so completion for variable
* names does not include the leading '$'. The $input parameter
* contains only the text following the '$' and therefore the
* candidates we return must do likewise...
*
* 5. ...Except when we are returning ALL variables (amongst other
* things) as candidates for completing the empty string '', in
* which case we DO need to include the '$' character in each of
* our candidates, because it was not already present in the text.
* (Note that $input will be '' when we are completing either ''
* or '$', so we need to distinguish between those two cases.)
*
* 6. Only a sub-set of other PHP operators constitute (or end with)
* word breaks, and so inconsistent behaviour can be expected if
* operators are used without surrounding whitespace to ensure a
* word break has occurred.
*
* Operators which DO break words include: '>' '<' '<<' '>>' '<>'
* '=' '==' '===' '!=' '!==' '>=' '<=' '<=>' '->' '|' '||' '&' '&&'
* '+=' '-=' '*=' '/=' '.=' '%=' '&=' '|=' '^=' '>>=' '<<=' '??='
*
* Operators which do NOT break words include: '!' '+' '-' '*' '/'
* '++' '--' '**' '%' '.' '~' '^' '??' '? :' '::'
*
* E.g.: although "foo()+bar()" is valid PHP, we would be unable
* to use completion to obtain the function name "bar" in that
* situation, as the $input string would actually begin with ")+"
* and the Matcher in question would not be returning candidates
* with that prefix.
*
* @see self::processCallback()
* @see \Psy\Shell::getTabCompletions()
*/
public const WORD_BREAK_CHARS = " \t\n\"\\’‘@$><=;|&{(";
phil-s marked this conversation as resolved.
Show resolved Hide resolved

/**
* A regular expression based on WORD_BREAK_CHARS which will match the
* completable word at the end of the string.
*/
public const WORD_REGEXP = "/[^ \t\n\"\\\\’‘@$><=;|&{(]*$/";

/**
* Register a tab completion Matcher.
*
Expand All @@ -42,7 +120,13 @@ public function activate()
}

/**
* Handle readline completion.
* Handle readline completion for the $input parameter (word).
*
* @see WORD_BREAK_CHARS
*
* @TODO: Post-process the completion candidates returned by each
* Matcher to ensure that they use the common prefix established by
* the $input parameter.
*
* @param string $input Readline current word
* @param int $index Current word index
Expand All @@ -64,11 +148,44 @@ public function processCallback($input, $index, $info = [])

$tokens = \token_get_all('<?php '.$line);

// remove whitespaces
// Remove whitespace tokens, excluding the current token.
$token = \array_pop($tokens);
$tokens = \array_filter($tokens, function ($token) {
return !AbstractMatcher::tokenIs($token, AbstractMatcher::T_WHITESPACE);
});

// If we're effectively completing an empty string, append a
// token to represent this. An empty string isn't actually a
// valid token, so we use the value that is simplest to test.
switch (true) {
case AbstractMatcher::tokenIs($token, AbstractMatcher::T_WHITESPACE):
$tokens[] = '';
break;
case AbstractMatcher::tokenIs($token, AbstractMatcher::T_VARIABLE):
case $token === '$':
// We allow a special case for '$', which for completion
// purposes we will treat the same way as T_VARIABLE.
$tokens[] = $token;
break;
case !AbstractMatcher::tokenIsValidIdentifier($token):
// This also covers/includes the cases !is_array($token)
// (which is a super-set of AbstractMatcher::isOperator()),
// and also tokenIs($token, AbstractMatcher::T_OPEN_TAG).
// Therefore it will never be the case that one of those
// things is the final token in the array; they can only
// ever be the *previous* token. Moreover, the final token
// is always a valid completion prefix or else the empty
// string. This simplifies the set of cases that Matchers
// need to cater for.
$tokens[] = $token;
$tokens[] = '';
break;
default:
// We're completing a valid identifier.
$tokens[] = $token;
break;
}

$matches = [];
foreach ($this->matchers as $matcher) {
if ($matcher->hasMatched($tokens)) {
Expand Down
18 changes: 16 additions & 2 deletions src/TabCompletion/Matcher/AbstractContextAwareMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,24 @@ protected function getVariable($var)
/**
* Get all variables in the current Context.
*
* @param bool $dollarPrefix
* Whether to prefix '$' to each variable name.
*
* @return array
*/
protected function getVariables()
protected function getVariables($dollarPrefix = false)
Copy link
Owner

Choose a reason for hiding this comment

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

This feels like an implementation detail for the VariablesMatcher. Will anything else ever need it? Should that matcher do the prefixing itself?

Copy link
Author

@phil-s phil-s Aug 17, 2020

Choose a reason for hiding this comment

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

I'm not sure whether anything else will ever use it; but if they do, I think it would be nicer if it was as simple as passing an argument to the existing function, so I'd be inclined to leave it where it is. (I'm happy to change it if you disagree, though.)

{
return $this->context->getAll();
$variables = $this->context->getAll();
if (!$dollarPrefix) {
return $variables;
}
else {
// Add '$' prefix to each name.
$newvars = [];
foreach ($variables as $name => $value) {
$newvars['$'.$name] = $value;
}
return $newvars;
}
}
}
Loading