Skip to content

Commit e104743

Browse files
committed
Fix 1-based index causing incorrect insertion of an empty word
Completing on a command line with a one-letter word at the end would incorrectly cause an empty word to be inserted before that character: ↙ cursor immediately after "a" (character 5) cmd a ----- \_ parsed to ['cmd', '', 'a'] instead of ['cmd', 'a'] This was due to `$this->charIndex` being zero-based, and `$cursor` being one-based; the condition `$cursor >= $this->charIndex` was always off by one. I'm have no idea what compelled me to write this with a one-based index in the first place, but all the tests are passing and I've tested this change in the console with a real application. It took a while to understand this function again, so I've put a few more helpful comments in for the next adventurer to pass by. Fixes #65
1 parent 3f9a6c0 commit e104743

File tree

3 files changed

+47
-7
lines changed

3 files changed

+47
-7
lines changed

src/CompletionContext.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ protected function splitCommand()
196196
{
197197
$this->words = array();
198198
$this->wordIndex = null;
199-
$cursor = 1;
199+
$cursor = 0;
200200

201201
$breaks = preg_quote($this->wordBreaks);
202202

@@ -211,21 +211,23 @@ protected function splitCommand()
211211
// Determine which word the cursor is in
212212
$cursor += strlen($wholeMatch);
213213
$word = $matches[1][$index];
214+
$breaks = $matches[2][$index];
214215

215216
if ($this->wordIndex === null && $cursor >= $this->charIndex) {
216217
$this->wordIndex = $index;
217218

218-
// Find the cursor position relative to the end of the word
219-
$cursorWordOffset = $this->charIndex - ($cursor - strlen($matches[2][$index]) - 1);
219+
// Find the user's cursor position relative to the end of this word
220+
// The end of the word is the internal cursor minus any break characters that were captured
221+
$cursorWordOffset = $this->charIndex - ($cursor - strlen($breaks));
220222

221223
if ($cursorWordOffset < 0) {
222224
// Cursor is inside the word - truncate the word at the cursor
223225
// (This emulates normal BASH completion behaviour I've observed, though I'm not entirely sure if it's useful)
224226
$word = substr($word, 0, strlen($word) + $cursorWordOffset);
225227

226228
} elseif ($cursorWordOffset > 0) {
227-
// Cursor is in the break-space after the word
228-
// Push an empty word at the cursor
229+
// Cursor is in the break-space after a word
230+
// Push an empty word at the cursor to allow completion of new terms at the cursor, ignoring words ahead
229231
$this->wordIndex++;
230232
$this->words[] = $word;
231233
$this->words[] = '';

src/CompletionHandler.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ protected function completeForOptions()
152152
{
153153
$word = $this->context->getCurrentWord();
154154

155-
if (strpos($word, '-') === 0) {
155+
if (substr($word, 0, 2) === '--') {
156156
$options = array();
157157

158158
foreach ($this->getAllOptions() as $opt) {

tests/Stecman/Component/Symfony/Console/BashCompletion/CompletionContextTest.php

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function testWordBreakSplit()
3131
public function testCursorPosition()
3232
{
3333
$context = new CompletionContext();
34-
$context->setCommandLine('make horse --legs 4 --colour black');
34+
$context->setCommandLine('make horse --legs 4 --colour black ');
3535

3636
// Cursor at the start of the line
3737
$context->setCharIndex(0);
@@ -42,6 +42,11 @@ public function testCursorPosition()
4242
$this->assertEquals(5, $context->getWordIndex());
4343
$this->assertEquals('black', $context->getCurrentWord());
4444

45+
// Cursor after space at the end of the string
46+
$context->setCharIndex(35);
47+
$this->assertEquals(6, $context->getWordIndex());
48+
$this->assertEquals('', $context->getCurrentWord());
49+
4550
// Cursor in the middle of 'horse'
4651
$context->setCharIndex(8);
4752
$this->assertEquals(1, $context->getWordIndex());
@@ -53,6 +58,39 @@ public function testCursorPosition()
5358
$this->assertEquals('--legs', $context->getCurrentWord());
5459
}
5560

61+
public function testWordBreakingWithSmallInputs()
62+
{
63+
$context = new CompletionContext();
64+
65+
// Cursor at the end of a word and not in the following space has no effect
66+
$context->setCommandLine('cmd a');
67+
$context->setCharIndex(5);
68+
$this->assertEquals(array('cmd', 'a'), $context->getWords());
69+
$this->assertEquals(1, $context->getWordIndex());
70+
$this->assertEquals('a', $context->getCurrentWord());
71+
72+
// As above, but in the middle of the command line string
73+
$context->setCommandLine('cmd a');
74+
$context->setCharIndex(3);
75+
$this->assertEquals(array('cmd', 'a'), $context->getWords());
76+
$this->assertEquals(0, $context->getWordIndex());
77+
$this->assertEquals('cmd', $context->getCurrentWord());
78+
79+
// Cursor at the end of the command line with a space appends an empty word
80+
$context->setCommandLine('cmd a ');
81+
$context->setCharIndex(8);
82+
$this->assertEquals(array('cmd', 'a', ''), $context->getWords());
83+
$this->assertEquals(2, $context->getWordIndex());
84+
$this->assertEquals('', $context->getCurrentWord());
85+
86+
// Cursor in break space before a word appends an empty word in that position
87+
$context->setCommandLine('cmd a');
88+
$context->setCharIndex(4);
89+
$this->assertEquals(array('cmd', '', 'a',), $context->getWords());
90+
$this->assertEquals(1, $context->getWordIndex());
91+
$this->assertEquals('', $context->getCurrentWord());
92+
}
93+
5694
public function testConfigureFromEnvironment()
5795
{
5896
putenv("CMDLINE_CONTENTS=beam up li");

0 commit comments

Comments
 (0)