Extend psql's \e and \ef commands so that a line number can be specified,

and the editor's cursor will be initially placed on that line.  In \e the
lines are counted with respect to the query buffer, while in \ef they are
counted with line 1 = first line of function body.  These choices are useful
for positioning the cursor on the line of a previously-reported error.

To avoid assumptions about what switch the user's editor takes for this
purpose, invent a new psql variable EDITOR_LINENUMBER_SWITCH with (at
present) no default value.

One incompatibility from previous behavior is that "\e 1234" will now
take "1234" as a line number not a file name.  There are at least two
ways to select a numerically-named file if you really want to.

Pavel Stehule, reviewed by Jan Urbanski, with further editing by Robert Haas
and Tom Lane

3 files changed, 190 insertions, 25 deletions

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 0887c705f52..655d3f890c6 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.330 2010/08/03 19:24:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.331 2010/08/12 00:40:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1620,6 +1620,10 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
  * pg_get_functiondef
  *		Returns the complete "CREATE OR REPLACE FUNCTION ..." statement for
  *		the specified function.
+ *
+ * Note: if you change the output format of this function, be careful not
+ * to break psql's rules (in \ef) for identifying the start of the function
+ * body.
  */
 Datum
 pg_get_functiondef(PG_FUNCTION_ARGS)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 90cd813f1ed..687cbca2ca6 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2000-2010, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.225 2010/08/03 18:33:09 tgl Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.226 2010/08/12 00:40:59 tgl Exp $
  */
 #include "postgres_fe.h"
 #include "command.h"
@@ -57,11 +57,12 @@ static backslashResult exec_command(const char *cmd,
 			 PsqlScanState scan_state,
 			 PQExpBuffer query_buf);
 static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
-		bool *edited);
+		int lineno, bool *edited);
 static bool do_connect(char *dbname, char *user, char *host, char *port);
 static bool do_shell(const char *command);
 static bool lookup_function_oid(PGconn *conn, const char *desc, Oid *foid);
 static bool get_create_function_cmd(PGconn *conn, Oid oid, PQExpBuffer buf);
+static int	strip_lineno_from_funcdesc(char *func);
 static void minimal_error_message(PGresult *res);
 
 static void printSSLInfo(void);
@@ -497,8 +498,8 @@ exec_command(const char *cmd,
 
 
 	/*
-	 * \e or \edit -- edit the current query buffer (or a file and make it the
-	 * query buffer
+	 * \e or \edit -- edit the current query buffer, or edit a file and make
+	 * it the query buffer
 	 */
 	else if (strcmp(cmd, "e") == 0 || strcmp(cmd, "edit") == 0)
 	{
@@ -510,17 +511,51 @@ exec_command(const char *cmd,
 		else
 		{
 			char	   *fname;
+			char	   *ln = NULL;
+			int			lineno = -1;
 
 			fname = psql_scan_slash_option(scan_state,
 										   OT_NORMAL, NULL, true);
-			expand_tilde(&fname);
 			if (fname)
-				canonicalize_path(fname);
-			if (do_edit(fname, query_buf, NULL))
-				status = PSQL_CMD_NEWEDIT;
-			else
-				status = PSQL_CMD_ERROR;
-			free(fname);
+			{
+				/* try to get separate lineno arg */
+				ln = psql_scan_slash_option(scan_state,
+											OT_NORMAL, NULL, true);
+				if (ln == NULL)
+				{
+					/* only one arg; maybe it is lineno not fname */
+					if (fname[0] &&
+						strspn(fname, "0123456789") == strlen(fname))
+					{
+						/* all digits, so assume it is lineno */
+						ln = fname;
+						fname = NULL;
+					}
+				}
+			}
+			if (ln)
+			{
+				lineno = atoi(ln);
+				if (lineno < 1)
+				{
+					psql_error("invalid line number: %s\n", ln);
+					status = PSQL_CMD_ERROR;
+				}
+			}
+			if (status != PSQL_CMD_ERROR)
+			{
+				expand_tilde(&fname);
+				if (fname)
+					canonicalize_path(fname);
+				if (do_edit(fname, query_buf, lineno, NULL))
+					status = PSQL_CMD_NEWEDIT;
+				else
+					status = PSQL_CMD_ERROR;
+			}
+			if (fname)
+				free(fname);
+			if (ln)
+				free(ln);
 		}
 	}
 
@@ -530,6 +565,8 @@ exec_command(const char *cmd,
 	 */
 	else if (strcmp(cmd, "ef") == 0)
 	{
+		int			lineno = -1;
+
 		if (!query_buf)
 		{
 			psql_error("no query buffer\n");
@@ -542,7 +579,13 @@ exec_command(const char *cmd,
 
 			func = psql_scan_slash_option(scan_state,
 										  OT_WHOLE_LINE, NULL, true);
-			if (!func)
+			lineno = strip_lineno_from_funcdesc(func);
+			if (lineno == 0)
+			{
+				/* error already reported */
+				status = PSQL_CMD_ERROR;
+			}
+			else if (!func)
 			{
 				/* set up an empty command to fill in */
 				printfPQExpBuffer(query_buf,
@@ -563,6 +606,32 @@ exec_command(const char *cmd,
 				/* error already reported */
 				status = PSQL_CMD_ERROR;
 			}
+			else if (lineno > 0)
+			{
+				/*
+				 * lineno "1" should correspond to the first line of the
+				 * function body.  We expect that pg_get_functiondef() will
+				 * emit that on a line beginning with "AS $function", and that
+				 * there can be no such line before the real start of the
+				 * function body.  Increment lineno by the number of lines
+				 * before that line, so that it becomes relative to the first
+				 * line of the function definition.
+				 */
+				const char *lines = query_buf->data;
+
+				while (*lines != '\0')
+				{
+					if (strncmp(lines, "AS $function", 12) == 0)
+						break;
+					lineno++;
+					/* find start of next line */
+					lines = strchr(lines, '\n');
+					if (!lines)
+						break;
+					lines++;
+				}
+			}
+
 			if (func)
 				free(func);
 		}
@@ -571,7 +640,7 @@ exec_command(const char *cmd,
 		{
 			bool		edited = false;
 
-			if (!do_edit(0, query_buf, &edited))
+			if (!do_edit(NULL, query_buf, lineno, &edited))
 				status = PSQL_CMD_ERROR;
 			else if (!edited)
 				puts(_("No changes"));
@@ -1543,11 +1612,11 @@ UnsyncVariables(void)
  * If you do not specify a filename, the current query buffer will be copied
  * into a temporary one.
  */
-
 static bool
-editFile(const char *fname)
+editFile(const char *fname, int lineno)
 {
 	const char *editorName;
+	const char *editor_lineno_switch = NULL;
 	char	   *sys;
 	int			result;
 
@@ -1562,6 +1631,26 @@ editFile(const char *fname)
 	if (!editorName)
 		editorName = DEFAULT_EDITOR;
 
+	/* Get line number switch, if we need it. */
+	if (lineno > 0)
+	{
+		editor_lineno_switch = GetVariable(pset.vars,
+										   "EDITOR_LINENUMBER_SWITCH");
+		if (editor_lineno_switch == NULL)
+		{
+			psql_error("EDITOR_LINENUMBER_SWITCH variable must be set to specify a line number\n");
+			return false;
+		}
+	}
+
+	/* Allocate sufficient memory for command line. */
+	if (lineno > 0)
+		sys = pg_malloc(strlen(editorName)
+						+ strlen(editor_lineno_switch) + 10	/* for integer */
+						+ 1 + strlen(fname) + 10 + 1);
+	else
+		sys = pg_malloc(strlen(editorName) + strlen(fname) + 10 + 1);
+
 	/*
 	 * On Unix the EDITOR value should *not* be quoted, since it might include
 	 * switches, eg, EDITOR="pico -t"; it's up to the user to put quotes in it
@@ -1569,11 +1658,20 @@ editFile(const char *fname)
 	 * severe brain damage in their command shell plus the fact that standard
 	 * program paths include spaces.
 	 */
-	sys = pg_malloc(strlen(editorName) + strlen(fname) + 10 + 1);
 #ifndef WIN32
-	sprintf(sys, "exec %s '%s'", editorName, fname);
+	if (lineno > 0)
+		sprintf(sys, "exec %s %s%d '%s'",
+				editorName, editor_lineno_switch, lineno, fname);
+	else
+		sprintf(sys, "exec %s '%s'",
+				editorName, fname);
 #else
-	sprintf(sys, SYSTEMQUOTE "\"%s\" \"%s\"" SYSTEMQUOTE, editorName, fname);
+	if (lineno > 0)
+		sprintf(sys, SYSTEMQUOTE "\"%s\" %s%d \"%s\"" SYSTEMQUOTE, 
+				editorName, editor_lineno_switch, lineno, fname);
+	else
+		sprintf(sys, SYSTEMQUOTE "\"%s\" \"%s\"" SYSTEMQUOTE,
+				editorName, fname);
 #endif
 	result = system(sys);
 	if (result == -1)
@@ -1588,7 +1686,8 @@ editFile(const char *fname)
 
 /* call this one */
 static bool
-do_edit(const char *filename_arg, PQExpBuffer query_buf, bool *edited)
+do_edit(const char *filename_arg, PQExpBuffer query_buf,
+		int lineno, bool *edited)
 {
 	char		fnametmp[MAXPGPATH];
 	FILE	   *stream = NULL;
@@ -1680,7 +1779,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, bool *edited)
 
 	/* call editor */
 	if (!error)
-		error = !editFile(fname);
+		error = !editFile(fname, lineno);
 
 	if (!error && stat(fname, &after) != 0)
 	{
@@ -2209,6 +2308,68 @@ get_create_function_cmd(PGconn *conn, Oid oid, PQExpBuffer buf)
 }
 
 /*
+ * If the given argument of \ef ends with a line number, delete the line
+ * number from the argument string and return it as an integer.  (We need
+ * this kluge because we're too lazy to parse \ef's function name argument
+ * carefully --- we just slop it up in OT_WHOLE_LINE mode.)
+ *
+ * Returns -1 if no line number is present, 0 on error, or a positive value
+ * on success.
+ */
+static int
+strip_lineno_from_funcdesc(char *func)
+{
+	char	   *c;
+	int			lineno;
+
+	if (!func || func[0] == '\0')
+		return -1;
+
+	c = func + strlen(func) - 1;
+
+	/*
+	 * This business of parsing backwards is dangerous as can be in a
+	 * multibyte environment: there is no reason to believe that we are
+	 * looking at the first byte of a character, nor are we necessarily
+	 * working in a "safe" encoding.  Fortunately the bitpatterns we are
+	 * looking for are unlikely to occur as non-first bytes, but beware
+	 * of trying to expand the set of cases that can be recognized.  We must
+	 * guard the <ctype.h> macros by using isascii() first, too.
+	 */
+
+	/* skip trailing whitespace */
+	while (c > func && isascii(*c) && isspace(*c))
+		c--;
+
+	/* must have a digit as last non-space char */
+	if (c == func || !isascii(*c) || !isdigit(*c))
+		return -1;
+
+	/* find start of digit string */
+	while (c > func && isascii(*c) && isdigit(*c))
+		c--;
+
+	/* digits must be separated from func name by space or closing paren */
+	/* notice also that we are not allowing an empty func name ... */
+	if (c == func || !isascii(*c) || !(isspace(*c) || *c == ')'))
+		return -1;
+
+	/* parse digit string */
+	c++;
+	lineno = atoi(c);
+	if (lineno < 1)
+	{
+		psql_error("invalid line number: %s\n", c);
+		return 0;
+	}
+
+	/* strip digit string from func */
+	*c = '\0';
+
+	return lineno;
+}
+
+/*
  * Report just the primary error; this is to avoid cluttering the output
  * with, for instance, a redisplay of the internally generated query
  */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 8351c5fb02b..69a073a2b39 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2000-2010, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/bin/psql/help.c,v 1.160 2010/07/20 03:54:19 rhaas Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/help.c,v 1.161 2010/08/12 00:40:59 tgl Exp $
  */
 #include "postgres_fe.h"
 
@@ -162,7 +162,7 @@ slashUsage(unsigned short int pager)
 {
 	FILE	   *output;
 
-	output = PageOutput(87, pager);
+	output = PageOutput(89, pager);
 
 	/* if you add/remove a line here, change the row count above */
 
@@ -174,8 +174,8 @@ slashUsage(unsigned short int pager)
 	fprintf(output, "\n");
 
 	fprintf(output, _("Query Buffer\n"));
-	fprintf(output, _("  \\e [FILE]              edit the query buffer (or file) with external editor\n"));
-	fprintf(output, _("  \\ef [FUNCNAME]         edit function definition with external editor\n"));
+	fprintf(output, _("  \\e [FILE] [LINE]       edit the query buffer (or file) with external editor\n"));
+	fprintf(output, _("  \\ef [FUNCNAME [LINE]]  edit function definition with external editor\n"));
 	fprintf(output, _("  \\p                     show the contents of the query buffer\n"));
 	fprintf(output, _("  \\r                     reset (clear) the query buffer\n"));
 #ifdef USE_READLINE