portmaster completions spew errors if there are no matches

[asomers]

The portmaster.fish completion script includes a shell glob based on the current commandline. When there are no matches, it prints a nasty error message, like this:

portmaster rhaerhlseugtralseugralegr<TAB>No matches for wildcard '/usr/ports/(commandline -ct)*/'.  (Tip: empty matches are allowed in 'set', 'count', 'for'.)
fish:           ls -d /usr/ports/(commandline -ct)*/                    | sed -E -e 's#/usr/ports/##' -e 's#/+#/#' -e 's#([^/]+/[^/]+).*#\1#'
                      ^
in command substitution
        called on standard input

It should be fixed to silently handle the case of no matches.
fish version 2.5.0, on FreeBSD 12, in QTerminal.

[asomers]

@frenchie Could you please look at this? I believe you wrote the original portmaster completions.

[krader1961]

In fairness to @frenchie this probably worked fine in older versions of fish and was only broken when we made it an error if a wildcard expansion failed outside of a few contexts.

[krader1961]

The trivial fix is to move the wildcard expansion to a set command then use the resulting variable in the subsequent ls -d command. However, in looking at that code there are other improvements possible.

[asomers]

@krader1961 I tried that but couldn't get it to work. How about this instead?

--- a/share/completions/portmaster.fish
+++ b/share/completions/portmaster.fish
@@ -48,8 +48,7 @@ complete -c portmaster -l version --description 'display the version number El E
 # Grab items from the ports directory, max depth 2
 complete -c portmaster -f --description 'Ports Directory' -a "
 (
-        ls -d /usr/ports/(commandline -ct)*/ \
-                | sed -E -e 's#/usr/ports/##' -e 's#/+#/#' -e 's#([^/]+/[^/]+).*#\1#'
+       __fish_complete_directories /usr/ports/(commandline -ct) "" | sed -E -e 's#/usr/ports/##g' -e 's#([^/]*/[^/]*)/[^/]*/#\1#'
 )"
 
 complete -c portmaster -f --description 'Installed Package' -a "(__fish_print_packages)"

[krader1961]

I tried that but couldn't get it to work.

What exactly did you try? It should be sufficient to replace the original statement with

set -l port_dirs /usr/ports/(commandline -ct)*/
echo $port_dirs | sed -E -e 's#/usr/ports/##' -e 's#/+#/#' -e 's#([^/]+/[^/]+).*#\1#'

Although, as I said earlier, this can be greatly improved given current fish capabilities. For example, it should be possible to replace the external sed command with the string builtin.

P.S., Note that the ls -d in the original code is completely superfluous and can be replaced with a simple echo.

[krader1961]

Sorry, my previous comment was wrong. It should have said

printf '%s\n' $port_dirs | sed -E -e 's#/usr/ports/##' -e 's#/+#/#' -e 's#([^/]+/[^/]+).*#\1#'

That's because sed works on lines and the original statement expects one path name per line.

[faho]

So, if I understand correclty, that completion wants to print directories in /usr/ports, two levels deep.

I.e. the sed expressions are

's#/usr/ports/##'

Removes the "/usr/ports" part.

's#/+#/#'

Replaces any multiple-slash runs with a single slash - which I would actually assume to be unnecessary (they can only come from the commandline token), and if the completion doesn't match the token anymore then we won't use it, which makes it actually wrong.

's#([^/]+/[^/]+).*#\1#'

Picks the first two path components (after /usr/ports/, since we just removed that).

---

So, how about this code

set -l dirs /usr/ports/*/*/ # will only generate directories because of the trailing slash
string replace '/usr/ports/' '' -- $dirs

?

We actually don't need the token here since fish will only pick matching completions anyway. Unless the amount of completions generated here is overwhelming (e.g. it takes too long to generate them), in which case we could just generate one level deep unless a token has been given.

[asomers]

@faho it's a little more complicated than that. The completion wants to expand to print directories in /usr/ports, up to two levels deep, but no deeper than the current commandline. So if the command line is at net- the completion should print net-im net-mgmt net-p2p. If the command line is at net-im/ then the completion should print all 197 directories beneath /usr/ports/net-im/. If the command line is at net-im/licq then the completion should print net-im/licq net-im/licq-jabber net-im/licq-osd net-im/licq-icq net-im/licq-msn net-im/licq-qt-gui. And it the commandline is at net-im/licq/ then the completion should refuse to expand further.

As you guessed, it's not ok to expand /usr/ports/*/* because it's too slow. There are about 27,000 directories, and on my system it takes > 30s list them all using NFS.

@krader1961 I'm not at my home PC right now so I can't tell you exactly what I tried. But suffice to say that several variations on "set -l foo ...; echo foo | sed ..." didn't work when used in my completion file, even though they seemed to work on the commandline. Most of the errors were something about passing the wrong number of arguments to "-a" for the complete command. Are there any differences in how fish executes code in a completion file vs interactively?

Nonetheless, the code I pasted above based on __fish_complete_directories seems to work. Does that look ok to everyone?

[faho]

As you guessed, it's not ok to expand /usr/ports// because it's too slow. There are about 27,000 directories, and on my system it takes > 30s list them all using NFS.

Ah, okay. In that case my plan B applies. Something like

set -l dirs /usr/ports/(commandline -ct)*/
set dirs $dirs $dirs/*/ #expand up to the second level, but if the token already has a "/" this won't match anything. In that case it will already have happened above.
# Alternatively, we could explicitly check for a "/" in the token.
# Deeper levels will be removed by the second replace below
string replace '/usr/ports/' '' -- $dirs | string replace -r '^([^/]+/[^/]+)/.*' '$1'

The regexes here are pretty much a literal translation of the sed script, minus the squashing of slash-runs.

Nonetheless, the code I pasted above based on __fish_complete_directories seems to work. Does that look ok to everyone?

Well, we'd usually prefer our builtin string over external commands. It's often faster, and it makes more stuff work without needing specific external commands (sometimes even specific versions - when I write sed stuff, I have no idea if it works on BSD or macOS, except for what I can glean from a man page). It also makes our code a bit more uniform and allows us to figure out issues in string.