Thread: about EDITOR_LINENUMBER_SWITCH
I noticed the 9.1 release notes claim that the new EDITOR_LINENUMBER_SWITCH thing is an environment variable, whereas it is actually a psql variable. This is perhaps sort of a Freudian slip. Since the editor itself is configured using an environment variable, shouldn't any configuration about the editor also be an environment variable, so people can configure them together? Another thought is that this whole thing could be done away with if we just allowed people to pass through arbitrary options to the editor, like \edit file.sql +50 -a -b -c For powerusers, this could have interesting possibilities.
On Sat, May 21, 2011 at 5:47 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > I noticed the 9.1 release notes claim that the new > EDITOR_LINENUMBER_SWITCH thing is an environment variable, whereas it is > actually a psql variable. > > This is perhaps sort of a Freudian slip. Since the editor itself is > configured using an environment variable, shouldn't any configuration > about the editor also be an environment variable, so people can > configure them together? It's probably the result of drift between the original patch and what was eventually committed. IIRC, Pavel had it as an environment variable originally, but Tom and I didn't feel the feature was important enough to merit that treatment. > Another thought is that this whole thing could be done away with if we > just allowed people to pass through arbitrary options to the editor, > like > > \edit file.sql +50 -a -b -c > > For powerusers, this could have interesting possibilities. That's an intriguing possibility. But part of the point of the original feature was to be able to say: \ef somefunc 10 ...and end up on line 10 of somefunc, perhaps in response to an error message complaining about that line. I don't think your proposal would address that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2011/5/21 Peter Eisentraut <peter_e@gmx.net>: > I noticed the 9.1 release notes claim that the new > EDITOR_LINENUMBER_SWITCH thing is an environment variable, whereas it is > actually a psql variable. > > This is perhaps sort of a Freudian slip. Since the editor itself is > configured using an environment variable, shouldn't any configuration > about the editor also be an environment variable, so people can > configure them together? > > Another thought is that this whole thing could be done away with if we > just allowed people to pass through arbitrary options to the editor, > like > > \edit file.sql +50 -a -b -c in original patch I had to do some magic operation with line number, so I had to know, what is a line number. A idea with other options are interesting. More usable can be store these option inside psql variable (be consistent with current state). Maybe in EDITOR_OPTIONS ? With possibility to overwrite this options from metacommand \edit file.sql 10 -x -y -z Regards Pavel Stehule > > For powerusers, this could have interesting possibilities. > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
On lör, 2011-05-21 at 20:39 -0400, Robert Haas wrote: > On Sat, May 21, 2011 at 5:47 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > > I noticed the 9.1 release notes claim that the new > > EDITOR_LINENUMBER_SWITCH thing is an environment variable, whereas it is > > actually a psql variable. > It's probably the result of drift between the original patch and what > was eventually committed. IIRC, Pavel had it as an environment > variable originally, but Tom and I didn't feel the feature was > important enough to merit that treatment. I think it's not really a matter of "importance", it's a matter of making things work correctly. I have a shell configuration that sets different environment variables, including editor, depending on what directory I'm in. Now I think that all the editors in question use the + syntax, but anyone else with something like that slightly out of the ordinary would be stuck. The other problem is if I change the editor here, I have to change this other piece there. Note that you cannot even specify the editor itself in psqlrc. > > Another thought is that this whole thing could be done away with if we > > just allowed people to pass through arbitrary options to the editor, > > like > > > > \edit file.sql +50 -a -b -c > > > > For powerusers, this could have interesting possibilities. > > That's an intriguing possibility. But part of the point of the > original feature was to be able to say: > > \ef somefunc 10 > > ...and end up on line 10 of somefunc, perhaps in response to an error > message complaining about that line. I don't think your proposal > would address that. Well, you'd write \ef somefunc +10 instead. Or something else, depending on the editor, but then you'd know what to write, since under the current theory you'd have to have configured it previously. Using the "+10" syntax also looks a bit clearer, in my mind.
On sön, 2011-05-22 at 06:30 +0200, Pavel Stehule wrote: > A idea with other options are > interesting. More usable can be store these option inside psql > variable (be consistent with current state). Maybe in > EDITOR_OPTIONS ? There isn't really a need for that, since if you want to pass options to your editor, you can stick them in the EDITOR variable. The idea would be more to pass options per occasion.
On Tue, May 24, 2011 at 4:36 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> That's an intriguing possibility. But part of the point of the >> original feature was to be able to say: >> >> \ef somefunc 10 >> >> ...and end up on line 10 of somefunc, perhaps in response to an error >> message complaining about that line. I don't think your proposal >> would address that. > > Well, you'd write > > \ef somefunc +10 > > instead. But that would not put you on line 10 of the function. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, May 24, 2011 at 4:36 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
>>> That's an intriguing possibility. But part of the point of the
>>> original feature was to be able to say:
>>>
>>> \ef somefunc 10
>>>
>>> ...and end up on line 10 of somefunc, perhaps in response to an error
>>> message complaining about that line. I don't think your proposal
>>> would address that.
>> Well, you'd write
>>
>> \ef somefunc +10
>>
>> instead.
> But that would not put you on line 10 of the function.
Right. It would also increase the cognitive load on the user to have
to remember the command-line go-to-line-number switch for his editor.
So I don't particularly want to redesign this feature. However, I can
see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from
the same place that you set EDITOR, which would suggest that we allow
the value to come from an environment variable. I'm not sure whether
there is merit in allowing both that source and ~/.psqlrc, though
possibly for Windows users it might be easier if ~/.psqlrc worked.
regards, tom lane
Robert Haas <robertmhaas@gmail.com> writes: > On Sat, May 21, 2011 at 5:47 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> I noticed the 9.1 release notes claim that the new >> EDITOR_LINENUMBER_SWITCH thing is an environment variable, whereas it is >> actually a psql variable. >> This is perhaps sort of a Freudian slip. > It's probably the result of drift between the original patch and what > was eventually committed. IIRC, Pavel had it as an environment > variable originally, but Tom and I didn't feel the feature was > important enough to merit that treatment. BTW, the above is merest historical revisionism: there was never a version of the patch that did it that way. AFAICS the idea started here: http://archives.postgresql.org/pgsql-hackers/2010-08/msg00089.php to which you immediately asked whether it should be an environmental variable, and I said no on what might be considered thin grounds: http://archives.postgresql.org/pgsql-hackers/2010-08/msg00182.php I can't see any real objection other than complexity to having it look for a psql variable and then an environment variable. Or we could drop the psql variable part of that, if it seems too complicated. Also, while we're on the subject, I'm not real sure why we don't allow the code to provide a default value when EDITOR has a well-known value like "vi" or "emacs". As long as there is a way to override that, where's the harm in a default? regards, tom lane
On Tue, May 24, 2011 at 5:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Sat, May 21, 2011 at 5:47 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >>> I noticed the 9.1 release notes claim that the new >>> EDITOR_LINENUMBER_SWITCH thing is an environment variable, whereas it is >>> actually a psql variable. >>> This is perhaps sort of a Freudian slip. > >> It's probably the result of drift between the original patch and what >> was eventually committed. IIRC, Pavel had it as an environment >> variable originally, but Tom and I didn't feel the feature was >> important enough to merit that treatment. > > BTW, the above is merest historical revisionism: there was never a > version of the patch that did it that way. Even if you were correct, that's a snarky way to put it, and the point is trivial anyway. But I don't think I'm imagining the getenv() call in this version of the patch: http://archives.postgresql.org/pgsql-hackers/2010-07/msg01253.php > Also, while we're on the subject, I'm not real sure why we don't allow > the code to provide a default value when EDITOR has a well-known value > like "vi" or "emacs". As long as there is a way to override that, > where's the harm in a default? Well, the question is how many people it'll help. Some people might have a full pathname, others might called it vim... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: > Right. It would also increase the cognitive load on the user to have > to remember the command-line go-to-line-number switch for his editor. > So I don't particularly want to redesign this feature. However, I can > see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from > the same place that you set EDITOR, which would suggest that we allow > the value to come from an environment variable. I'm not sure whether > there is merit in allowing both that source and ~/.psqlrc, though > possibly for Windows users it might be easier if ~/.psqlrc worked. If we're going to increase the number of options in .psqlrc that do not work with older psql versions, can I please have .psqlrc-MAJORVERSION or some such? Having 8.3's psql complain all the time because it doesn't understand "linestyle" is annoying. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011:
>> Right. It would also increase the cognitive load on the user to have
>> to remember the command-line go-to-line-number switch for his editor.
>> So I don't particularly want to redesign this feature. However, I can
>> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from
>> the same place that you set EDITOR, which would suggest that we allow
>> the value to come from an environment variable. I'm not sure whether
>> there is merit in allowing both that source and ~/.psqlrc, though
>> possibly for Windows users it might be easier if ~/.psqlrc worked.
> If we're going to increase the number of options in .psqlrc that do not
> work with older psql versions, can I please have .psqlrc-MAJORVERSION or
> some such? Having 8.3's psql complain all the time because it doesn't
> understand "linestyle" is annoying.
1. I thought we already did have that.
2. In any case, EDITOR_LINENUMBER_SWITCH isn't a hazard for this,
because older versions will just think it's a variable without any
special meaning.
But the real question here is whether we want to change it to be also
(or instead?) an environment variable.
regards, tom lane
Excerpts from Tom Lane's message of mié may 25 16:07:55 -0400 2011: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: > >> Right. It would also increase the cognitive load on the user to have > >> to remember the command-line go-to-line-number switch for his editor. > >> So I don't particularly want to redesign this feature. However, I can > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from > >> the same place that you set EDITOR, which would suggest that we allow > >> the value to come from an environment variable. I'm not sure whether > >> there is merit in allowing both that source and ~/.psqlrc, though > >> possibly for Windows users it might be easier if ~/.psqlrc worked. > > > If we're going to increase the number of options in .psqlrc that do not > > work with older psql versions, can I please have .psqlrc-MAJORVERSION or > > some such? Having 8.3's psql complain all the time because it doesn't > > understand "linestyle" is annoying. > > 1. I thought we already did have that. Oh, true, we have that, though it's not very usable because you have to rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, which is kinda silly. > 2. In any case, EDITOR_LINENUMBER_SWITCH isn't a hazard for this, > because older versions will just think it's a variable without any > special meaning. Good point. > But the real question here is whether we want to change it to be also > (or instead?) an environment variable. I vote yes. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Here's a patch to fix what has been discussed:
* Change EDITOR_LINENUMBER_SWITCH to environment variable.
* I also changed "switch" to "arg" because "switch" is a bit of a
sloppy term.
* So the environment variable is called
PSQL_EDITOR_LINENUMBER_ARG.
* Set "+" as hardcoded default value on Unix (since "vi" is the
hardcoded default editor), so many users won't have to configure
this at all.
* I moved the documentation around a bit to centralize the editor
configuration under environment variables, rather than repeating
bits of it under every backslash command that invoked an editor.
Attachment
Alvaro Herrera wrote: > Excerpts from Tom Lane's message of mié may 25 16:07:55 -0400 2011: > > Alvaro Herrera <alvherre@commandprompt.com> writes: > > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: > > >> Right. It would also increase the cognitive load on the user to have > > >> to remember the command-line go-to-line-number switch for his editor. > > >> So I don't particularly want to redesign this feature. However, I can > > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from > > >> the same place that you set EDITOR, which would suggest that we allow > > >> the value to come from an environment variable. I'm not sure whether > > >> there is merit in allowing both that source and ~/.psqlrc, though > > >> possibly for Windows users it might be easier if ~/.psqlrc worked. > > > > > If we're going to increase the number of options in .psqlrc that do not > > > work with older psql versions, can I please have .psqlrc-MAJORVERSION or > > > some such? Having 8.3's psql complain all the time because it doesn't > > > understand "linestyle" is annoying. > > > > 1. I thought we already did have that. > > Oh, true, we have that, though it's not very usable because you have to > rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > which is kinda silly. True. We don't add configuration changes in minor versions so having minor-version granularity makes no sense. The attached patch changes this to use the _major_ version number for psql rc files. Does this have to be backward-compatible? Should I check for minor and major matches? That is going to be confusing to document. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml new file mode 100644 index 662eab7..1ea728b *** a/doc/src/sgml/ref/psql-ref.sgml --- b/doc/src/sgml/ref/psql-ref.sgml *************** PSQL_EDITOR_LINENUMBER_ARG='--line ' *** 3332,3338 **** Both the system-wide <filename>psqlrc</filename> file and the user's <filename>~/.psqlrc</filename> file can be made version-specific by appending a dash and the <productname>PostgreSQL</productname> ! release number, for example <filename>~/.psqlrc-&version;</filename>. A matching version-specific file will be read in preference to a non-version-specific file. </para> --- 3332,3338 ---- Both the system-wide <filename>psqlrc</filename> file and the user's <filename>~/.psqlrc</filename> file can be made version-specific by appending a dash and the <productname>PostgreSQL</productname> ! major release number, for example <filename>~/.psqlrc-9.2</filename>. A matching version-specific file will be read in preference to a non-version-specific file. </para> diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c new file mode 100644 index 3c17eec..9670b7e *** a/src/bin/psql/startup.c --- b/src/bin/psql/startup.c *************** process_psqlrc_file(char *filename) *** 600,607 **** #define R_OK 4 #endif ! psqlrc = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1); ! sprintf(psqlrc, "%s-%s", filename, PG_VERSION); if (access(psqlrc, R_OK) == 0) (void) process_file(psqlrc, false, false); --- 600,607 ---- #define R_OK 4 #endif ! psqlrc = pg_malloc(strlen(filename) + 1 + strlen(PG_MAJORVERSION) + 1); ! sprintf(psqlrc, "%s-%s", filename, PG_MAJORVERSION); if (access(psqlrc, R_OK) == 0) (void) process_file(psqlrc, false, false);
On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian <bruce@momjian.us> wrote: > Alvaro Herrera wrote: >> Excerpts from Tom Lane's message of mié may 25 16:07:55 -0400 2011: >> > Alvaro Herrera <alvherre@commandprompt.com> writes: >> > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: >> > >> Right. It would also increase the cognitive load on the user to have >> > >> to remember the command-line go-to-line-number switch for his editor. >> > >> So I don't particularly want to redesign this feature. However, I can >> > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from >> > >> the same place that you set EDITOR, which would suggest that we allow >> > >> the value to come from an environment variable. I'm not sure whether >> > >> there is merit in allowing both that source and ~/.psqlrc, though >> > >> possibly for Windows users it might be easier if ~/.psqlrc worked. >> > >> > > If we're going to increase the number of options in .psqlrc that do not >> > > work with older psql versions, can I please have .psqlrc-MAJORVERSION or >> > > some such? Having 8.3's psql complain all the time because it doesn't >> > > understand "linestyle" is annoying. >> > >> > 1. I thought we already did have that. >> >> Oh, true, we have that, though it's not very usable because you have to >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, >> which is kinda silly. > > True. We don't add configuration changes in minor versions so having > minor-version granularity makes no sense. > > The attached patch changes this to use the _major_ version number for > psql rc files. Does this have to be backward-compatible? Should I > check for minor and major matches? That is going to be confusing to > document. Checking for a minor match and then a major match seems sensible. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian <bruce@momjian.us> wrote: > > Alvaro Herrera wrote: > >> Excerpts from Tom Lane's message of mi? may 25 16:07:55 -0400 2011: > >> > Alvaro Herrera <alvherre@commandprompt.com> writes: > >> > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: > >> > >> Right. ?It would also increase the cognitive load on the user to have > >> > >> to remember the command-line go-to-line-number switch for his editor. > >> > >> So I don't particularly want to redesign this feature. ?However, I can > >> > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from > >> > >> the same place that you set EDITOR, which would suggest that we allow > >> > >> the value to come from an environment variable. ?I'm not sure whether > >> > >> there is merit in allowing both that source and ~/.psqlrc, though > >> > >> possibly for Windows users it might be easier if ~/.psqlrc worked. > >> > > >> > > If we're going to increase the number of options in .psqlrc that do not > >> > > work with older psql versions, can I please have .psqlrc-MAJORVERSION or > >> > > some such? ?Having 8.3's psql complain all the time because it doesn't > >> > > understand "linestyle" is annoying. > >> > > >> > 1. I thought we already did have that. > >> > >> Oh, true, we have that, though it's not very usable because you have to > >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > >> which is kinda silly. > > > > True. ?We don't add configuration changes in minor versions so having > > minor-version granularity makes no sense. > > > > The attached patch changes this to use the _major_ version number for > > psql rc files. ?Does this have to be backward-compatible? ?Should I > > check for minor and major matches? ?That is going to be confusing to > > document. > > Checking for a minor match and then a major match seems sensible. Done, and documented in the attached patch. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml new file mode 100644 index 662eab7..4eefe3b *** a/doc/src/sgml/ref/psql-ref.sgml --- b/doc/src/sgml/ref/psql-ref.sgml *************** PSQL_EDITOR_LINENUMBER_ARG='--line ' *** 3332,3339 **** Both the system-wide <filename>psqlrc</filename> file and the user's <filename>~/.psqlrc</filename> file can be made version-specific by appending a dash and the <productname>PostgreSQL</productname> ! release number, for example <filename>~/.psqlrc-&version;</filename>. ! A matching version-specific file will be read in preference to a non-version-specific file. </para> </listitem> --- 3332,3341 ---- Both the system-wide <filename>psqlrc</filename> file and the user's <filename>~/.psqlrc</filename> file can be made version-specific by appending a dash and the <productname>PostgreSQL</productname> ! major or minor release number, for example ! <filename>~/.psqlrc-9.2</filename> or ! <filename>~/.psqlrc-9.2.5</filename>. The most specific ! version-matching file will be read in preference to a non-version-specific file. </para> </listitem> diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c new file mode 100644 index 3c17eec..71829eb *** a/src/bin/psql/startup.c --- b/src/bin/psql/startup.c *************** process_psqlrc(char *argv0) *** 594,613 **** static void process_psqlrc_file(char *filename) { ! char *psqlrc; #if defined(WIN32) && (!defined(__MINGW32__)) #define R_OK 4 #endif ! psqlrc = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1); ! sprintf(psqlrc, "%s-%s", filename, PG_VERSION); ! if (access(psqlrc, R_OK) == 0) ! (void) process_file(psqlrc, false, false); else if (access(filename, R_OK) == 0) (void) process_file(filename, false, false); ! free(psqlrc); } --- 594,620 ---- static void process_psqlrc_file(char *filename) { ! char *psqlrc_minor, *psqlrc_major; #if defined(WIN32) && (!defined(__MINGW32__)) #define R_OK 4 #endif ! psqlrc_minor = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1); ! sprintf(psqlrc_minor, "%s-%s", filename, PG_VERSION); ! psqlrc_major = pg_malloc(strlen(filename) + 1 + strlen(PG_MAJORVERSION) + 1); ! sprintf(psqlrc_major, "%s-%s", filename, PG_MAJORVERSION); ! /* check for minor version first, then major, then no version */ ! if (access(psqlrc_minor, R_OK) == 0) ! (void) process_file(psqlrc_minor, false, false); ! else if (access(psqlrc_major, R_OK) == 0) ! (void) process_file(psqlrc_major, false, false); else if (access(filename, R_OK) == 0) (void) process_file(filename, false, false); ! ! free(psqlrc_minor); ! free(psqlrc_major); }
Excerpts from Robert Haas's message of vie oct 14 09:36:47 -0300 2011: > > On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian <bruce@momjian.us> wrote: > > Alvaro Herrera wrote: > >> Oh, true, we have that, though it's not very usable because you have to > >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > >> which is kinda silly. > > > > True. We don't add configuration changes in minor versions so having > > minor-version granularity makes no sense. > > > > The attached patch changes this to use the _major_ version number for > > psql rc files. Does this have to be backward-compatible? Should I > > check for minor and major matches? That is going to be confusing to > > document. > > Checking for a minor match and then a major match seems sensible. And backwards compatible too! +1 to that. An idea that you can describe in six words doesn't seem all that confusing. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote: > > Excerpts from Robert Haas's message of vie oct 14 09:36:47 -0300 2011: > > > > On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian <bruce@momjian.us> wrote: > > > Alvaro Herrera wrote: > > > >> Oh, true, we have that, though it's not very usable because you have to > > >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > > >> which is kinda silly. > > > > > > True. We don't add configuration changes in minor versions so having > > > minor-version granularity makes no sense. > > > > > > The attached patch changes this to use the _major_ version number for > > > psql rc files. Does this have to be backward-compatible? Should I > > > check for minor and major matches? That is going to be confusing to > > > document. > > > > Checking for a minor match and then a major match seems sensible. > > And backwards compatible too! +1 to that. An idea that you can > describe in six words doesn't seem all that confusing. Oops, I see a problem. Right now, our first major release has no zero, e.g. 9.2, while our minors have a third digit, 9.2.5. The problem is that with this patch it is confusing to have a psql config file that matches 9.2.0, but not 9.2.5, because you can't write 9.2.0. A file called .psql-9.2 matches 9.2.0, but also matches 9.2.X if there is no matching minor release file. The bottom line is that with this patch, .psql-9.2 is both a minor and possibly minor matcher. I can't blame the patch, but rather our version numbering system. Prior to the patch 9.2 always meant just 9.2.0. This patch adds an additional confusion. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> Oops, I see a problem. Right now, our first major release has no zero,
> e.g. 9.2, while our minors have a third digit, 9.2.5. The problem is
> that with this patch it is confusing to have a psql config file that
> matches 9.2.0, but not 9.2.5, because you can't write 9.2.0.
Uh, this seems like nonsense. We've been labeling major releases with
a dot-zero for some time, and that's embedded in process now (cf
version_stamp.pl) to the point that we're unlikely to forget to do so.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Oops, I see a problem. Right now, our first major release has no zero, > > e.g. 9.2, while our minors have a third digit, 9.2.5. The problem is > > that with this patch it is confusing to have a psql config file that > > matches 9.2.0, but not 9.2.5, because you can't write 9.2.0. > > Uh, this seems like nonsense. We've been labeling major releases with > a dot-zero for some time, and that's embedded in process now (cf > version_stamp.pl) to the point that we're unlikely to forget to do so. Ah, I see now. 9.1 has: #define PG_VERSION "9.1.0" I rarely see the non-dev trees. No problem then. Patch applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On tor, 2011-10-13 at 11:31 -0400, Bruce Momjian wrote: > The attached patch changes this to use the _major_ version number for > psql rc files. Does this have to be backward-compatible? Should I > check for minor and major matches? That is going to be confusing to > document. Contrary to what the subject suggests, I think the main reason people wanted this feature was to be able to set the linestyle to unicode without getting a warning from older releases about unknown linestyle or something. But in a few years, they'll have to maintain .psqlrc-9.2, .psqlrc-9.3, .psqlrc-9.4, etc. That doesn't sound like a useful long-term solution either.
On 10/15/2011 09:37 AM, Peter Eisentraut wrote: > On tor, 2011-10-13 at 11:31 -0400, Bruce Momjian wrote: >> The attached patch changes this to use the _major_ version number for >> psql rc files. Does this have to be backward-compatible? Should I >> check for minor and major matches? That is going to be confusing to >> document. > Contrary to what the subject suggests, I think the main reason people > wanted this feature was to be able to set the linestyle to unicode > without getting a warning from older releases about unknown linestyle or > something. But in a few years, they'll have to > maintain .psqlrc-9.2, .psqlrc-9.3, .psqlrc-9.4, etc. That doesn't sound > like a useful long-term solution either. > > Wouldn't it be better to support some conditional syntax? cheers andrew
Peter Eisentraut wrote: > On tor, 2011-10-13 at 11:31 -0400, Bruce Momjian wrote: > > The attached patch changes this to use the _major_ version number for > > psql rc files. Does this have to be backward-compatible? Should I > > check for minor and major matches? That is going to be confusing to > > document. > > Contrary to what the subject suggests, I think the main reason people > wanted this feature was to be able to set the linestyle to unicode > without getting a warning from older releases about unknown linestyle or > something. But in a few years, they'll have to > maintain .psqlrc-9.2, .psqlrc-9.3, .psqlrc-9.4, etc. That doesn't sound > like a useful long-term solution either. Well, frankly, I think the fact we were matching on minor version number was even worse. This is slightly better. I guess they could use symlinks to keep a config file for multiple versions, but I agree it isn't a long-term great solution. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Andrew Dunstan wrote: > > > On 10/15/2011 09:37 AM, Peter Eisentraut wrote: > > On tor, 2011-10-13 at 11:31 -0400, Bruce Momjian wrote: > >> The attached patch changes this to use the _major_ version number for > >> psql rc files. Does this have to be backward-compatible? Should I > >> check for minor and major matches? That is going to be confusing to > >> document. > > Contrary to what the subject suggests, I think the main reason people > > wanted this feature was to be able to set the linestyle to unicode > > without getting a warning from older releases about unknown linestyle or > > something. But in a few years, they'll have to > > maintain .psqlrc-9.2, .psqlrc-9.3, .psqlrc-9.4, etc. That doesn't sound > > like a useful long-term solution either. > > > > > > Wouldn't it be better to support some conditional syntax? I suppose if we add that to psql we can remove this factility completely. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Peter Eisentraut <peter_e@gmx.net> writes:
> Contrary to what the subject suggests, I think the main reason people
> wanted this feature was to be able to set the linestyle to unicode
> without getting a warning from older releases about unknown linestyle or
> something. But in a few years, they'll have to
> maintain .psqlrc-9.2, .psqlrc-9.3, .psqlrc-9.4, etc. That doesn't sound
> like a useful long-term solution either.
Well, "in a few years" they won't need that conditionality any more at
all, so I'm not sure I believe the above argument. The problem seems
inherently self-limiting.
What struck me while looking at the patch is that it is conditional
based on *psql's* version. Not the version of the server you're
connected to. I'm not too sure what use-cases people have for version
dependence here, but I'd think that the server version would enter into
it sometimes.
(Of course, for server version to be used usefully, you'd need to
re-execute the rc file during \c, something we don't do now.)
regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > Contrary to what the subject suggests, I think the main reason people > > wanted this feature was to be able to set the linestyle to unicode > > without getting a warning from older releases about unknown linestyle or > > something. But in a few years, they'll have to > > maintain .psqlrc-9.2, .psqlrc-9.3, .psqlrc-9.4, etc. That doesn't sound > > like a useful long-term solution either. > > Well, "in a few years" they won't need that conditionality any more at > all, so I'm not sure I believe the above argument. The problem seems > inherently self-limiting. > > What struck me while looking at the patch is that it is conditional > based on *psql's* version. Not the version of the server you're > connected to. I'm not too sure what use-cases people have for version > dependence here, but I'd think that the server version would enter into > it sometimes. The assumption is that the .psqlrc is controlling psql behavior. Not sure what setting would be changed based on server version, maybe psql variables. I have updated the docs to indicate it is the psql version. > (Of course, for server version to be used usefully, you'd need to > re-execute the rc file during \c, something we don't do now.) Yep, yuck. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +