Path auto-complete stops working after directory with space

[aik099]

I have a command argument, that is set to be auto-completed with paths. When one of directories contains space in it, then it's suggested as \ and that's fine, but after that no directories are suggested.

Example:

svn-buddy log ~/web/p/1.\ stable/ 

Just press TAB after last / and no directories will be auto-completed.

[aik099]

@stecman , ping.

[stecman]

Hey @aik099, sorry for the wait on this, I've been really busy lately.

This is an issue because the word parsing in the current implementation just uses a regular expression to split the command line, so can't cleanly handle strings. Support for escaped spaces can be added easily:

diff --git a/src/CompletionContext.php b/src/CompletionContext.php
index 6029253..ff261aa 100644
--- a/src/CompletionContext.php
+++ b/src/CompletionContext.php
@@ -200,7 +200,7 @@ class CompletionContext

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

-        if (!preg_match_all("/([^$breaks]*)([$breaks]*)/", $this->commandLine, $matches)) {
+        if (!preg_match_all("/((?:\\\\.|[^$breaks])*)([$breaks]*)/", $this->commandLine, $matches)) {
             return;
         }

I've had a play with support for strings and escaped spaces and have it mostly working, but I'm not really happy with that implementation. It replaces the preg_match_all call in CompletionContext::splitCommand with a custom function...I think a better implementation would re-work splitCommand instead of imitating preg_match_all and using the existing implementation.

Feel free to have a crack at it if you like. I'm happy to merge the regex change above to add support for escaped spaces in the meantime if that'd be useful.

[aik099]

Hey @aik099, sorry for the wait on this, I've been really busy lately.

No worries. I've just discovered issue management features in https://github.com/ptsochantaris/trailer app I was using and looking through all issues I've ever reported that are not yet closed.

if (!preg_match_all("/((?:\\\\.|[^$breaks])*)([$breaks]*)/", $this->commandLine, $matches)) {

Why you're not using this expression and decided to change to splitCommand method?

[JoshuaBehrens]

Any plans on working on this bug? Are there any bugs in your gist that remained unsolved? If the gist is THE solution I'd like to see this commited. If there are still bugs in it, which ones. I am willing to help solving them to get whitespace support.

[stecman]

@JoshuaBehrens I had a look at this over the weekend, and that splitOnBreaks function in the Gist is pretty hard to follow coming in cold - probably why I was reluctant to use it.

I'll have a look at rewriting this tonight. It should be possible to implement this with regular expressions rather than resorting to hand-coding a lexer like I was doing for that prototype.

1. Something like this to tokenize into words, strings and non-words (breaks):

/(?:
    (?P<single_quote_string>
        '(\\.|[^'\\])*'
    ) |
    (?P<double_quote_string>
        "(\\.|[^"\\])*"
    ) |
    (?P<word>
        (?:\\.|[^\(\)=\ \t\n])*
    ) |
    (?P<break>
        [\(\)=\ \t\n]*
    )
)/x

2. Calculate wordIndex with the full length tokens that include start/end quotes and escape characters.

3. Remove escape characters and start/end quotes from the captured words (maybe - it's been a while since I worked on this :hammer: )

As an aside, it looks like we can remove ( and ) from the break characters along with ' and " for this change, since they don't make sense in this context - I suspect they're in Bash's word breaks list because they mark sub-shells and substitution.