From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Tab completion for ALTER TABLE =?utf-8?Q?=E2=80=A6?=
 ADD =?utf-8?Q?=E2=80=A6?=
References: <87bl6ehhpl.fsf@wibble.ilmari.org> <YShkMBE3sq8mDa1S@paquier.xyz>
	<87k0k7upla.fsf@wibble.ilmari.org> <YSwsmd44F3tjFyLI@paquier.xyz>
Date: Mon, 30 Aug 2021 14:38:19 +0100
In-Reply-To: <YSwsmd44F3tjFyLI@paquier.xyz> (Michael Paquier's message of
	"Mon, 30 Aug 2021 09:55:53 +0900")
Message-ID: <87h7f7uk6s.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: 
 <https://www.postgresql.org/message-id/87h7f7uk6s.fsf%40wibble.ilmari.org>
Precedence: bulk

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Michael Paquier <michael@paquier.xyz> writes:

> On Fri, Aug 27, 2021 at 11:52:33AM +0100, Dagfinn Ilmari Mannsåker wrote:
>> That was easy enough to add (just a bit of extra fiddling to handle
>> COLUMN being optional), done in the attached v2 patch.
>
> This part was a bit misleading, as it would recommend a list of types
> when specifying just ADD CONSTRAINT for example, so I have removed
> it.

That was because I forgot to exclude all the other object types that can
come after ADD.  Attached is a patch that does that.  I also moved it
right next to the ALTER TABLE … ADD completion, and added a comment to
keep the two lists in sync.

> An extra thing that felt a bit overdoing is the addition of KEY after
> PRIMARY/FOREIGN.

Yeah, I guess people are unlikely to write out the whole PRIMARY or FOREIGN
and only then hit tab to complete the rest.

>> Doing a list of arbitrarily many comma-separated names is more
>> complicated, so that can be the subject for another patch.
>
> No objections to that.  I have applied what we have now, as that's
> already an improvement.

Thanks!

- ilmari


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=0001-Complete-type-names-after-ALTER-TABLE-ADD-COLUMN.patch

From a5fac64310b01ed32e281031a2c274e835463853 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 30 Aug 2021 14:25:55 +0100
Subject: [PATCH 1/2] =?UTF-8?q?Complete=20type=20names=20after=20ALTER=20T?=
 =?UTF-8?q?ABLE=20=E2=80=A6=20ADD=20[COLUMN]=E2=80=88=E2=80=A6?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since COLUMN is optional, we need to keep the list of object types to
complete after ADD in sync with the list of words not to complete type
names after.  Add a comment to this effect.
---
 src/bin/psql/tab-complete.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7c6af435a9..197f4c736c 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2025,8 +2025,14 @@ psql_completion(const char *text, int start, int end)
 					  "DETACH PARTITION", "FORCE ROW LEVEL SECURITY");
 	/* ALTER TABLE xxx ADD */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ADD"))
+		/* make sure to keep this list and the !Matches() below in sync */
 		COMPLETE_WITH("COLUMN", "CONSTRAINT", "CHECK", "UNIQUE", "PRIMARY KEY",
 					  "EXCLUDE", "FOREIGN KEY");
+	/* ATER TABLE xxx ADD [COLUMN] yyy */
+	else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN", MatchAny) ||
+			 (Matches("ALTER", "TABLE", MatchAny, "ADD", MatchAny) &&
+			  !Matches("ALTER", "TABLE", MatchAny, "ADD", "COLUMN|CONSTRAINT|CHECK|UNIQUE|PRIMARY|EXCLUDE|FOREIGN")))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes, NULL);
 	/* ALTER TABLE xxx ADD CONSTRAINT yyy */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ADD", "CONSTRAINT", MatchAny))
 		COMPLETE_WITH("CHECK", "UNIQUE", "PRIMARY KEY", "EXCLUDE", "FOREIGN KEY");
-- 
2.30.2


--=-=-=--