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

Do not autocomplete if only whitespace precedes cursor #410

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
25 changes: 23 additions & 2 deletions src/Psy/TabCompletion/AutoCompleter.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public function processCallback($input, $index, $info = array())
$line = $input;
}

if ($this->shouldInsertOnlyTab($line, $info['point'])) {
return array("\t");
Copy link
Owner

Choose a reason for hiding this comment

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

because of the magic space inserted after tab completion, this inserts five spaces for the first tab stop, then nine spaces for each additional tab stop. why don't we hijack this and return three spaces? then it's always four space indents :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It inserts a tab character, not spaces. I'm definitely not a fan of replacing it with space characters, as those are different (no matter what you think of tabs vs spaces) and substituting one for the other silently can never be what you want.

The tab + space behaviour is sort of vexing, but it's not something I can realistically solve in a way that doesn't break everything in environments that do work properly (for example ones that have the path from php/php-src#2720 applied).

What I can probably do is return array("\t" . chr(8));. chr(8) is the backspace character, which should work.

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 could add in a setting for this and apply it to every autocompletion, thinking about it. Until the patch I mentioned works that's a sort of viable workaround to get autocompletion as you'd expect from an IDE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait no, that won't work. The space is appended after the completion - so a backspace that is inserted will actually just remove the tab character entirely.

Copy link
Owner

@bobthecow bobthecow Sep 13, 2017

Choose a reason for hiding this comment

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

i understand that tabs and spaces are different. but practically, one looks like you'd expect, and the other looks really bad. so ¯\_(ツ)_/¯

}

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

// remove whitespaces
Expand All @@ -81,13 +85,30 @@ public function processCallback($input, $index, $info = array())
return !empty($matches) ? $matches : array('');
}

/**
* @param string $line Readline current line
* @param int $point The cursor position on the line
*
* @return bool true if the characters before $point are only whitespace
*
* @internal param int $index Current word index
*/
private function shouldInsertOnlyTab($line, $point)
{
$precedingInput = substr($line, 0, $point);

$trimmedInput = trim($precedingInput);

return $trimmedInput === '';
}

/**
* The readline_completion_function callback handler.
*
* @see processCallback
*
* @param $input
* @param $index
* @param string $input
* @param int $index
*
* @return array
*/
Expand Down
3 changes: 2 additions & 1 deletion test/Psy/Test/TabCompletion/AutoCompleterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ public function testClassesCompletion($line, $mustContain, $mustNotContain)

$code = $tabCompletion->processCallback('', 0, array(
'line_buffer' => $line,
'point' => 0,
'start' => 0,
'point' => strlen($line),
'end' => strlen($line),
));

Expand Down