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

On Fri, Jul 18, 2025 at 12:49 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> On Thu, 17 Jul 2025 10:57:36 +0900
> Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> > On Tue, 17 Jun 2025 00:08:32 +0900
> > Yugo Nagata <nagata@sraoss.co.jp> wrote:
> >
> > > On Thu, 5 Jun 2025 16:52:00 +0900
> > > Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > >
> > > > On Thu, 5 Jun 2025 10:08:35 +0900
> > > > Yugo Nagata <nagata@sraoss.co.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:

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.


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'?

---
+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().

---
-   /* 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))

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com