sh(1): autocomplete commands

[pstef]

Summary

Without this patch, sh can autocomplete file names but not commands from
$PATH. Use libedit's facility to execute custom function for autocomplete,
but yield to the library's standard autocomplete function when cursor is
not at position 0.

Please note that for this to work as intended, libedit has to be synchronized
with upstream.

[arichardson]

Thanks for working on this, $PATH completion for commands is the feature I miss most when using /bin/sh.

bin/sh/histedit.c

77

The attribute is ignored in the declaration, it's only needed in the definition.

526

Does this function need a special case if text starts with /?

541

Could just declare char execpath[PATH_MAX] and avoid this malloc?

549

usr.bin/which.c has the following, so maybe we need to handle empty elements explicitly:

	if (strchr(filename, '/') != NULL)
		return (is_there(filename) ? 0 : -1);
	found = 0;
	while ((d = strsep(&path, ":")) != NULL) {
		if (*d == '\0')
			d = ".";
		if (snprintf(candidate, sizeof(candidate), "%s/%s", d,
		    filename) >= (int)sizeof(candidate))
			continue;
		if (is_there(candidate)) {
			found = 1;
			if (!allpaths)
				break;
		}
	}

570

missing closedir(dir)?

586

cdefs.h has a macro for this attribute, so it can be
sh_complete(EditLine *sel, int ch __unused)

[pstef]

Address mistakes pointed out by Alex.

[pstef]

Thanks, I tried to fix all the obvious mistakes. I will have to test and think about the rest of your comments.

bin/sh/histedit.c

580

This will have to be escaped since libedit will refuse to do it seeing that the completion function is non-standard.

[jilles]

Nice feature.

bin/sh/histedit.c

547

Code to retrieve all commands in PATH does not exist yet in sh, so it is appropriate to write it here.

Note that this does not include builtins or functions.

549

Handling empty elements is definitely required.

555–560

Using fstatat() will allow avoiding the string manipulation.

555–560

d_type could be used to reduce stat calls: if DT_REG the file can be accepted without stat call, if DT_UNKNOWN or DT_LNK a stat call is required, otherwise the file can be rejected without stat call.

564–565

The paranoid will want some overflow checks here.

570

The code in contrib/libedit/filecomplete.c will sort the list, but will not remove duplicates. Do we care?

580

To be precise, all names will have to be escaped. Otherwise, the single_match check in contrib/libedit/filecomplete.c may see a difference.

581

Is is possible that this overflows the allocated memory by one pointer?

[pstef]

Apply some of the fixes suggested by Jilles.

[pstef]

bin/sh/histedit.c

547

Do we want to autocomplete builtins and functions? If so, maybe in a later patch? :)

549

Do you mean the if (*d == '\0') d = "."; part? So that it will search for matching commands in $PWD?

570

Currently I don't see a good reason to sort the list, but I may change my mind.

580

It may be debatable. If only one command was found (so matches[2] is NULL) and we decide to escape only matches[0] and matches[1] then all the conditions for single_match == TRUE are met. Then libedit should replace user input with an escaped equivalent and skip showing matches.

For the multiple matches case we could escape all of them, so we would have the behavior described above and also every item on the list of suggestions would be escaped, like I've been seeing in some Linux distributions for a while (except that seems to be limited to enclosing the file name with single or double quotes; no backslashes).

[bapt]

When hitting twice tab in a crappy environnement the same path added multiple time in PATH, it proposes completion for a given file for every occurence.

When I hit tab twice, it proposes PATH and "file"

[pstef]

Add code to remove duplicates.

[pstef]

I believe that all reported issues (and more) have been addressed.

[arichardson]

bin/sh/histedit.c

73

I don't think this needs to be a global variable (see comments below).

532–537

533

624

If you use qsort_s here and pass (void *)(intptr_t)curpos as the extra argument you can avoid the global variable and make curpos local to sh_matches.

[bapt]

this patch has been tested successfully here, thank you for having worked on that.
as for the review of the code I don't have any further comments to add than the one which has already been provided.