Re: psql: tab-completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

On Wed, 17 Sep 2025 12:53:10 -0700
Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:

> On Fri, Jul 18, 2025 at 12:49 AM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> >
> > On Thu, 17 Jul 2025 10:57:36 +0900
> > Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> >
> > > On Tue, 17 Jun 2025 00:08:32 +0900
> > > Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> > >
> > > > On Thu, 5 Jun 2025 16:52:00 +0900
> > > > Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> > > >
> > > > > On Thu, 5 Jun 2025 10:08:35 +0900
> > > > > Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Currently, tab completion for COPY only suggests filenames after TO or
> > > > > > FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.
> > > > > >
> > > > > > I'd like to propose improving the tab completion behavior as described in
> > > > > > the subject, so that these keywords are suggested appropriately, and filenames
> > > > > > are offered as potential command names after the PROGRAM keyword.
> > > > > >
> > > > > > I've attached this proposal as a patch series with the following three parts:
> > > > >
> > > > > I'm sorry but the previous patches were accidentally broken and didn't work.
> > > > > I've attached fixed patches.
> > > > >
> > > > > >
> > > > > > 0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()
> > > > > >
> > > > > > This is a preparatory cleanup. Most completions in match_previous_words() already use
> > > > > > COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
> > > > > > still remain.
> > > > > >
> > > > > > This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
> > > > > > COMPLETE_WITH_GENERATOR, improving consistency and readability.
> > > > > >
> > > > > > 0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM
> > > > > >
> > > > > > This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
> > > > > > after TO or FROM. After PROGRAM, filenames are suggested as possible command names.
> > > > > >
> > > > > > To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
> > > > > > combining literal keywords with filename suggestions in the completion list.
> > > > > >
> > > > > > 0003: Improve tab completion for COPY option lists
> > > > > >
> > > > > > Currently, only the first option in a parenthesized list is suggested during completion,
> > > > > > and nothing is suggested after a comma.
> > > > > >
> > > > > > This patch enables suggestions after each comma, improving usability when specifying
> > > > > > multiple options.
> > > > > >
> > > > > > Although not directly related to the main proposal, I believe this is a helpful enhancement
> > > > > > to COPY tab completion and included it here for completeness.
> > > > > >
> > > > > > I’d appreciate your review and feedback on this series.
> >
> > The previous patch was broken failed to complie since I missed following
> > the required format of if-conditions in match_previous_words().
> > I've attached update patches.
> >
>
> I agree with the basic direction of the patches. Here are some
> comments on the first two patches:

Thank you for reviewing it.
I've attached an updated patch.

>
> v5-0001-Refactor-match_previous_words-to-remove-direct-us.patch:
>
> ---
> +#define COMPLETE_WITH_GENERATOR(function) \
> + matches = rl_completion_matches(text, function)
>
> I think it would be clearer if we use 'generator' or 'genfunc' instead
> of 'function' as a macro argument.

I fixed it to use 'generator'.

>
> v5-0002-Add-tab-completion-support-for-COPY-.-TO-FROM-STD.patch:
>
> ---
> + /* Complete COPY|\copy <sth> FROM|TO PROGRAM command */
> + else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", "PROGRAM"))
> + COMPLETE_WITH_FILES("", HeadMatches("COPY")); /*
> COPY requires quoted filename */
>
> Why does it complete the query with files names even after 'PROGRAM'?

Users can specify the command by giving a filename with an absolute or
relative path, so I think it makes sense to allow filename completion
after PROGRAM.

> ---
> +static char *
> +_complete_from_files(const char *text, int state)
> {
>
> I think the comments of complete_from_files() should be moved to this
> new function. For instance, the comments starts with:
>
> * This function wraps rl_filename_completion_function() to strip quotes from
> * the input before searching for matches and to quote any matches for which
> * the consuming command will require it.
>
> But complete_from_files() function no longer calls
> rl_filename_completion_function().

I moved the comments to the top of _complete_from_files() and added a new
comment for complete_from_files() to describe that it is a wrapper of the
former.

> ---
> - /* Complete COPY <sth> FROM <sth> */
> - else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
> + /* Complete COPY <sth> FROM [PROGRAM] <sth> */
> + else if (Matches("COPY|\\copy", MatchAny, "FROM",
> MatchAnyExcept("PROGRAM")) ||
> + Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))
>
> I see this kind of conversion many places in the patch; convert one
> condition with MatchAny into two conditions with
> MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
> simplifying it using MatchAnyN. For example,
>
> else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>