Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hKlG6-0000fE-KK for pgsql-hackers@arkaria.postgresql.org; Sun, 28 Apr 2019 14:58:26 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hKlG4-0008EO-36 for pgsql-hackers@arkaria.postgresql.org; Sun, 28 Apr 2019 14:58:24 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hKlG3-0008EH-MD for pgsql-hackers@lists.postgresql.org; Sun, 28 Apr 2019 14:58:23 +0000 Received: from xvm-110-146.dc2.ghst.net ([46.226.110.146] helo=fetter.org) by makus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hKlFw-0006IV-82 for pgsql-hackers@postgresql.org; Sun, 28 Apr 2019 14:58:22 +0000 Received: by fetter.org (Postfix, from userid 1001) id 073BA40E58; Sun, 28 Apr 2019 16:58:14 +0200 (CEST) Date: Sun, 28 Apr 2019 16:58:13 +0200 From: David Fetter To: Fabien COELHO Cc: Corey Huinker , PostgreSQL Development Subject: [PATCH v4] Add \warn to psql Message-ID: <20190428145813.GO28936@fetter.org> References: <20190421183115.GA4311@fetter.org> <20190422134532.GV28936@fetter.org> <20190427171548.GM28936@fetter.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="X1xGqyAVbSpAWs5A" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --X1xGqyAVbSpAWs5A Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sat, Apr 27, 2019 at 10:09:27PM +0200, Fabien COELHO wrote: > > Hello David, > > About v3. Applies, compiles, global & local make check are ok. doc gen ok. > > > > I'd put the commands in alphabetical order (echo, qecho, warn) instead of > > > e/w/q in the condition. > > > > Done. > > Cannot see it: > > + else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "warn") == 0 || strcmp(cmd, "qecho") == 0) My mistake. I didn't think the order in which they were compared mattered much, but it makes sense on further reflection to keep things tidy in the code. > > > The -n trick does not appear in the help lines, ISTM that it could fit, so > > > maybe it could be added, possibly something like: > > > > > > \echo [-n] [TEXT] write string to stdout, possibly without trailing newline > > > > > > and same for \warn and \qecho? > > > > Makes sense, but I put it there just for \echo to keep lines short. > > I think that putting together the 3 echo variants help makes sense, but > maybe someone will object about breaking the abc order. Here's the alphabetical version. > > > > How might we test this portably? > > > > > > Hmmm... TAP tests are expected to be portable. Attached a simple POC, which > > > could be extended to test many more things which are currently out of > > > coverage (src/bin/psql stuff is covered around 40% only). > > > > Thanks for putting this together. I've added this test, and agree that > > increasing coverage is important for another patch. > > Yep. Speaking of which, I'd like to see about getting your patch against Testlib.pm in so more tests of psql can also go in. It's not a new feature /per se/, and it doesn't break any current scripts, so I'd make the argument that it's OK for them to go in and possibly even be back-patched. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate --X1xGqyAVbSpAWs5A Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0001-Add-warn-to-psql.patch" From 76ce9a85f92ec4f2283ee967b3067e3f3efda232 Mon Sep 17 00:00:00 2001 From: David Fetter Date: Sun, 21 Apr 2019 11:08:40 -0700 Subject: [PATCH v4] Add \warn to psql To: hackers MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------2.20.1" This is a multi-part message in MIME format. --------------2.20.1 Content-Type: text/plain; charset=UTF-8; format=fixed Content-Transfer-Encoding: 8bit - Does what it says on the label - Adds TAP tests for same - In passing, expose the -n option for \echo, \qecho, and \warn in \? output create mode 100644 src/bin/psql/t/001_psql.pl --------------2.20.1 Content-Type: text/x-patch; name="v4-0001-Add-warn-to-psql.patch" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="v4-0001-Add-warn-to-psql.patch" diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index b86764003d..4edf8e67a1 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1879,6 +1879,26 @@ Tue Oct 26 21:40:57 CEST 1999 + + \warn text [ ... ] + + + Prints the arguments to the standard error, separated by one + space and followed by a newline. This can be useful to + intersperse information in the output of scripts when not polluting + standard output is desired. For example: + + +=> \echo :variable +=> \warn `date` +Sun Apr 21 10:48:11 PDT 2019 + + If the first argument is an unquoted -n the trailing + newline is not written. + + + + \ef function_description line_number diff --git a/src/bin/psql/.gitignore b/src/bin/psql/.gitignore index c2862b12d6..d324c1c1fa 100644 --- a/src/bin/psql/.gitignore +++ b/src/bin/psql/.gitignore @@ -3,3 +3,4 @@ /sql_help.c /psql +/tmp_check diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile index 69bb297fe7..9473ab01cb 100644 --- a/src/bin/psql/Makefile +++ b/src/bin/psql/Makefile @@ -60,8 +60,15 @@ uninstall: clean distclean: rm -f psql$(X) $(OBJS) lex.backup + rm -rf tmp_check # files removed here are supposed to be in the distribution tarball, # so do not clean them in the clean/distclean rules maintainer-clean: distclean rm -f sql_help.h sql_help.c psqlscanslash.c + +check: + $(prove_check) + +installcheck: + $(prove_installcheck) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 8254d61099..d4846ce729 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -319,7 +319,7 @@ exec_command(const char *cmd, status = exec_command_ef_ev(scan_state, active_branch, query_buf, true); else if (strcmp(cmd, "ev") == 0) status = exec_command_ef_ev(scan_state, active_branch, query_buf, false); - else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0) + else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0 || strcmp(cmd, "warn") == 0) status = exec_command_echo(scan_state, active_branch, cmd); else if (strcmp(cmd, "elif") == 0) status = exec_command_elif(scan_state, cstack, query_buf); @@ -1114,7 +1114,7 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch, } /* - * \echo and \qecho -- echo arguments to stdout or query output + * \echo, \warn and \qecho -- echo arguments to stdout or query output */ static backslashResult exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd) @@ -1129,6 +1129,8 @@ exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd) if (strcmp(cmd, "qecho") == 0) fout = pset.queryFout; + else if (strcmp(cmd, "warn") == 0) + fout = stderr; else fout = stdout; diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index d6d41b51d5..bc98f01be7 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -206,11 +206,12 @@ slashUsage(unsigned short int pager) fprintf(output, _("Input/Output\n")); fprintf(output, _(" \\copy ... perform SQL COPY with data stream to the client host\n")); - fprintf(output, _(" \\echo [STRING] write string to standard output\n")); + fprintf(output, _(" \\echo [-n] [STRING] write string to standard output (-n for no newline)\n")); fprintf(output, _(" \\i FILE execute commands from file\n")); fprintf(output, _(" \\ir FILE as \\i, but relative to location of current script\n")); + fprintf(output, _(" \\qecho [-n] [STRING] write string to query output stream (-n for no newline, see \\o)\n")); fprintf(output, _(" \\o [FILE] send all query results to file or |pipe\n")); - fprintf(output, _(" \\qecho [STRING] write string to query output stream (see \\o)\n")); + fprintf(output, _(" \\warn [-n] [STRING] write string to standard error (-n for no newline)\n")); fprintf(output, "\n"); fprintf(output, _("Conditional\n")); diff --git a/src/bin/psql/t/001_psql.pl b/src/bin/psql/t/001_psql.pl new file mode 100644 index 0000000000..32dd43279b --- /dev/null +++ b/src/bin/psql/t/001_psql.pl @@ -0,0 +1,32 @@ +use strict; +use warnings; + +use PostgresNode; +use TestLib; +use Test::More; + +my $node = get_new_node('main'); +$node->init(); +$node->start(); + +# invoke psql +# - opts: space-separated options and arguments +# - stat: expected exit status +# - in: input stream +# - out: list of re to check on stdout +# - err: list of re to check on stderr +# - name: of the test +sub psql +{ + local $Test::Builder::Level = $Test::Builder::Level + 1; + my ($opts, $stat, $in, $out, $err, $name) = @_; + my @cmd = ('psql', split /\s+/, $opts); + $node->command_checks_all(\@cmd, $stat, $out, $err, $name, $in); + return; +} + +psql('-c \\q', 0, '', [ qr{^$} ], [ qr{^$} ], 'psql -c'); +psql('', 0,"\\echo hello\n\\warn world\n\\q\n", [ qr{^hello$} ], [ qr{^world$} ], 'psql in/out/err'); + +$node->stop(); +done_testing(); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index bcddc7601e..383a17a7f4 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1421,7 +1421,7 @@ psql_completion(const char *text, int start, int end) "\\t", "\\T", "\\timing", "\\unset", "\\x", - "\\w", "\\watch", + "\\w","\\warn", "\\watch", "\\z", "\\!", "\\?", NULL diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index a164cdbd8c..6ad7f681ae 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -519,22 +519,24 @@ sub command_fails_like } # Run a command and check its status and outputs. -# The 5 arguments are: +# The 5 to 6 arguments are: # - cmd: ref to list for command, options and arguments to run # - ret: expected exit status # - out: ref to list of re to be checked against stdout (all must match) # - err: ref to list of re to be checked against stderr (all must match) # - test_name: name of test +# - in: standard input sub command_checks_all { local $Test::Builder::Level = $Test::Builder::Level + 1; - my ($cmd, $expected_ret, $out, $err, $test_name) = @_; + my ($cmd, $expected_ret, $out, $err, $test_name, $in) = @_; + $in = '' if not defined $in; # run command my ($stdout, $stderr); print("# Running: " . join(" ", @{$cmd}) . "\n"); - IPC::Run::run($cmd, '>', \$stdout, '2>', \$stderr); + IPC::Run::run($cmd, '<', \$in, '>', \$stdout, '2>', \$stderr); # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR my $ret = $?; --------------2.20.1-- --X1xGqyAVbSpAWs5A--