Re: [psql] Setting the PROMPT on command line

Hi there,

I wrote a tiny patch to prevent psql from overwriting the PROMPT[1-3]
variables with default values after parsing the command line. That way
the prompt can be set from the command line by using:

psql -v PROMPT1=...

The patch is against postgres 7.4.5, but it's so trivial it should be
possible to apply it to CVS without changes.

Cheers,
Ingo

On Thu, 2004-10-07 at 10:04, Ingo van Lil wrote:
> I wrote a tiny patch to prevent psql from overwriting the PROMPT[1-3]
> variables with default values after parsing the command line.

Looks good to me. Does anyone object to applying this for 8.0?

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> On Thu, 2004-10-07 at 10:04, Ingo van Lil wrote:
>> I wrote a tiny patch to prevent psql from overwriting the PROMPT[1-3]
>> variables with default values after parsing the command line.

> Looks good to me. Does anyone object to applying this for 8.0?

I'm just wondering if the fix is in the right place. Doesn't the same
issue arise for other variables? Maybe what we have to do is postpone
command-line-driven settings of variables until sometime later than it's
done now.

regards, tom lane

On Thu, 2004-10-07 at 13:05, Tom Lane wrote:
> I'm just wondering if the fix is in the right place. Doesn't the same
> issue arise for other variables?

Well, we don't manually set other variables in psql itself, with the
exception of defining some default values before we process command-line
args or psqlrc (see startup.c circa line 141) -- so in that case
allowing the value to be overriden is fine.

On looking at the code closer though, I do see one related problem: we
process command-line args before we read psqlrc, so psqlrc settings can
override what was specified on the command-line. Shouldn't this happen
in reverse?

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> On Thu, 2004-10-07 at 13:05, Tom Lane wrote:
>> I'm just wondering if the fix is in the right place. Doesn't the same
>> issue arise for other variables?

> Well, we don't manually set other variables in psql itself, with the
> exception of defining some default values before we process command-line
> args or psqlrc (see startup.c circa line 141) -- so in that case
> allowing the value to be overriden is fine.

[ eyeballs that... ] So a much simpler fix for Ingo's complaint would
be to move the default settings of PROMPT1 et al (lines 303-306) to the
vicinity of line 141. I can't see that skipping these setup operations
in the noninteractive case is worthwhile.

> On looking at the code closer though, I do see one related problem: we
> process command-line args before we read psqlrc, so psqlrc settings can
> override what was specified on the command-line. Shouldn't this happen
> in reverse?

I tend to agree, but it's somewhat debatable. Would it be simple to
change?

regards, tom lane

On Thu, 2004-10-07 at 13:46, Tom Lane wrote:
> [ eyeballs that... ] So a much simpler fix for Ingo's complaint would
> be to move the default settings of PROMPT1 et al (lines 303-306) to the
> vicinity of line 141.

Indeed, that is true -- that's the better fix IMHO. Barring any
objections I'll commit this fix before end-of-day today.

> > On looking at the code closer though, I do see one related problem: we
> > process command-line args before we read psqlrc, so psqlrc settings can
> > override what was specified on the command-line. Shouldn't this happen
> > in reverse?
[...]
> Would it be simple to change?

The main complication is that there is a command-line argument that
instructs psql not to read psqlrc; that means the easy fix of just
reading psqlrc before processing command-line arguments won't work.
Still shouldn't be too complicated to fix (e.g. postpone the -v settings
until we've processed psqlrc but do the rest of the command-line arg
processing immediately), but I'm content to leave it for 8.1

-Neil

On Thu, 2004-10-07 at 15:15, Neil Conway wrote:
> Indeed, that is true -- that's the better fix IMHO. Barring any
> objections I'll commit this fix before end-of-day today.

Patch applied -- apologies for the delay.

-Neil