Re: [PATCH 3/3] programs/cmd: implement filename completion

[Jacek Caban]

Hi Eric,

On 3/24/22 14:46, Eric Pouech wrote:
> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com>
>
> ---
>   programs/cmd/batch.c |  279 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 273 insertions(+), 6 deletions(-)
>
> diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c
> index 9a262c5fec5..934cbcc7a39 100644
> --- a/programs/cmd/batch.c
> +++ b/programs/cmd/batch.c
> @@ -227,6 +227,211 @@ WCHAR *WCMD_parameter (WCHAR *s, int n, WCHAR **start, BOOL raw,
>     return WCMD_parameter_with_delims (s, n, start, raw, wholecmdline, L" \t,=;");
>   }
>
> +struct completion_info
> +{
> +    WCHAR** subfiles;
> +    DWORD index_search;
> +    DWORD num_results;
> +};
> +
> +/* gets directory name out of 'path', appends 'in' and surround with quotes if needed */
> +static WCHAR* dup_quoted(const WCHAR* path, const WCHAR* in)
> +{
> +    const WCHAR* last = wcsrchr(path, L'/');
> +    size_t dirlen;
> +    size_t len;
> +    WCHAR* ret;
> +
> +    if (last)
> +    {
> +        WCHAR* last1 = wcschr(last + 1, L'\\');
> +        if (last1) last = last1;
> +    }
> +    else last = wcsrchr(path, L'\\');
> +    dirlen = last ? (last - path) + 1 : 0;

I think that last1 should use wcsrchr as well? Or maybe even replace the 
whole dirlen logic with a simple loop.
> +static BOOL init_completion(struct completion_info* ci, const WCHAR* from, DWORD len, BOOL forward)
> +{
> +    WCHAR* ptr;
> +    HANDLE hff;
> +    WIN32_FIND_DATAW fd;
> +    BOOL in_quotes = FALSE;
> +    DWORD i, j = 0;
> +
> +    ptr = malloc((len + 1 + 1) * sizeof(WCHAR));
> +    if (!ptr) return FALSE;
> +    for (i = 0; i < len; i++)
> +    {
> +        switch (from[i])
> +        {
> +        case L'"':
> +            in_quotes = !in_quotes;
> +            continue;
> +        case L'\\':
> +            if (!in_quotes && i + 1 < len && wcschr(L" \t\"\\", from[i + 1]) != NULL)
> +                ++i;
> +            break;
> +        case L' ':
> +        case L'\t':
> +            /* shouldn't happen, as 'from' should contain a single argument */
> +            if (!in_quotes)
> +            {
> +                free(ptr);
> +                return FALSE;
> +            }
> +            break;
> +        }
> +        ptr[j++] = from[i];
> +    }
> +    ptr[j] = L'*';
> +    ptr[j + 1] = L'\0';
> +    hff = FindFirstFileW(ptr, &fd);
> +    ci->num_results = 0;
> +    ci->subfiles = NULL;
> +    if (hff != INVALID_HANDLE_VALUE)
> +    {
> +        do
> +        {
> +            WCHAR** new;
> +            if (!wcscmp(fd.cFileName, L".") || !wcscmp(fd.cFileName, L".."))
> +                continue;
> +            new = realloc(ci->subfiles, (ci->num_results + 1) * sizeof(*ci->subfiles));
> +            if (!new)
> +            {
> +                FindClose(hff);
> +                free(ptr);
> +                return FALSE;
> +            }
> +            ci->subfiles = new;
> +            if (!(ci->subfiles[ci->num_results++] = dup_quoted(ptr, fd.cFileName)))
> +            {
> +                FindClose(hff);
> +                free(ptr);
> +                return FALSE;
> +            }
> +        } while (FindNextFileW(hff, &fd));
> +        FindClose(hff);
> +    }
> +    free(ptr);
> +    if (!ci->num_results) return FALSE;
> +    ci->index_search = forward ? 0 : ci->num_results - 1;
> +    return TRUE;
> +}

If I read it correctly, if another process creates a file when 
completion is already initialized, we will not take that into account. 
Is that intended?


Thanks,

Jacek