Fish completion fails with quotes in argument

[Luap99]

The fish completion script prints ugly traces if quotes are not balanced in the argument.

To reproduce: ./testprog '[TAB]

> ./testprog '- (line 1): Unexpected end of string, quotes are not balanced
./testprog __complete ' 
                      ^
in command substitution
        called on line 35 of file -
in function '__testprog_perform_completion' with arguments './testprog\ \''
        called on line 1 of file -
in command substitution
        called on line 71 of file -
in function '__testprog_prepare_completions'
in command substitution

This happens with single and double quotes. Technically the completion still works and offers file completion but the traces makes this unusable.

I also checked this with zsh, bash, #1146 and #1208 but only fish is affected. All other shells offer file completion, which is in my eyes the right thing.

[Luap99]

@marckhouzam PTAL

[github-actions]

This issue is being marked as stale due to a long period of inactivity

[marckhouzam]

Sorry @Luap99 I forgot about this one. I'll try to take a look soon.

[marckhouzam]

I'm finally looking into this and I believe the issue is that for fish, command errors cannot be re-directed. This means that if a command has unbalanced quoting, like our case, it will print an error.
See fish-shell/fish-shell#6991 for an explanation.

This seems to mean that to avoid the error printouts being visible, we would need to catch the case of unbalanced quoting ourselves, which I don't think is trivial (e.g, ./testprog 'TAB vs ./testprog ''TAB vs ./testprog ''compTAB' etc)

One idea I thought of is that we could execute the command not directly with fish but using sh. So instead of effectively having the completion script call

eval "testprog/bin/testprog __complete '" 2> /dev/null

which prints the error message
we could call

sh -c "testprog/bin/testprog __complete '" 2> /dev/null

which properly redirects the error output to /dev/null

We should maybe check for the presence of sh first? Which makes wonder if jumping through those hoops is worth it?

@Luap99 could you give some more info on when this situation may occur?

[Luap99]

Why not fish -c "testprog/bin/testprog __complete '" 2> /dev/null? This should work and I think it is safe to assume fish is in $PATH. Or maybe use $SHELL -c ...

@Luap99 could you give some more info on when this situation may occur?

I only hit this issue while testing. I don't think I would have encountered this problem in a normal use case. The fact that no one has opened a similar issue or commented on this one leads me to believe that it is not important to fix this problem.

[marckhouzam]

Why not fish -c "testprog/bin/testprog __complete '" 2> /dev/null? This should work and I think it is safe to assume fish is in $PATH. Or maybe use $SHELL -c ...

Hey that works (I didn't expect it to)! And I think that is safer than using sh.

In trying to test further I thought of trying completion when using:

$HOME/testprog [TAB]
~/testprog [TAB]

and these currently don't work for fish. This is because of the use of type -q in the script, which was a workaround to not being able to redirect error to /dev/null. So, now that we have a possible solution to the redirection, it suddenly becomes even more interesting to use it.

I'll open a new issue for this new bug and post a PR to address this one and the new one.

[marckhouzam]

$HOME/testprog [TAB]
~/testprog [TAB]

and these currently don't work for fish.

I've opened #1306

[marckhouzam]

I've added the discussed fix for this issue to #1249