Hello,

Happy New Year everyone,

Attached is an attempt to eliminate duplicate pg_dump
option descriptions, and have a single description for both
short and long options. For me, as for a translator, this
eliminates the need to maintain the two, exactly same, sets of 
24 sentences.

If this is accepted, using the same approach I'll go through pg_dumpall
and other tools, which suffer from the same exact problem.
If it's rejected, please advise of a better way to approach
the problem.

Needless to say, it is a pain for a translator to 
properly maintain the two sets of messages for every tool PG got.

After all that is settled, I'll send translation patches in.

Thank you,
-s

 pg_dump.options.diff
Thu, 2 Jan 2003 01:16:58 -0500
"Serguei Mokhov" <mokhov@cs.concordia.ca>
pg_dump.options.diff
"Serguei Mokhov" <mokhov@cs.concordia.ca> writes:
> Attached is an attempt to eliminate duplicate pg_dump
> option descriptions, and have a single description for both
> short and long options. For me, as for a translator, this
> eliminates the need to maintain the two, exactly same, sets of 
> 24 sentences.

Offhand, this cure strikes me as much worse than the disease.  You've
converted code which was understandable, if somewhat repetitious, into
code that can be understood by neither programmers nor translators.
The text strings have been broken into fragments that don't make any
sense individually --- which probably creates translating problems,
as well as opportunities for programmer error.

I see your complaint, but this doesn't seem like a good way to fix it.

Perhaps it would work better to do something like

#ifdef HAVE_GETOPT_LONG
	char* f_option = _("-f, --file=FILENAME     ");
	... etc ...
#else /* not HAVE_GETOPT_LONG */
	char* f_option = _("-f FILENAME             ");
	... etc ...
#endif /* not HAVE_GETOPT_LONG */

	printf(_("  %s output file name\n"), f_option);
	... etc ...

That seems to reduce the amount of duplication without breaking things
up into chunks that aren't independent concepts.

However, I'm not convinced that the above is better than what we have
--- it's really not obvious that the above is more maintainable than

#ifdef HAVE_GETOPT_LONG
	printf(_("  -f, --file=FILENAME      output file name\n"));
#else /* not HAVE_GETOPT_LONG */
	printf(_("  -f FILENAME              output file name\n"));
#endif /* not HAVE_GETOPT_LONG */

There are worse things than a little repetitiveness, and creating
the opportunity to mismatch a flag with its description may well
be one of them.

Comments?  Can anyone do better?

			regards, tom lane

Thu, 02 Jan 2003 01:34:23 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump.options.diff 
----- Original Message ----- 
From: "Tom Lane" <tgl@sss.pgh.pa.us>
Sent: January 02, 2003 1:34 AM

> Perhaps it would work better to do something like
> 
> #ifdef HAVE_GETOPT_LONG
> char* f_option = _("-f, --file=FILENAME     ");
> ... etc ...
> #else /* not HAVE_GETOPT_LONG */
> char* f_option = _("-f FILENAME             ");
> ... etc ...
> #endif /* not HAVE_GETOPT_LONG */
> 
> printf(_("  %s output file name\n"), f_option);
> ... etc ...
> 
> That seems to reduce the amount of duplication without breaking things
> up into chunks that aren't independent concepts.

Thank you for your comment, Tom.

A slightly more readable version of the above could be:

> #ifdef HAVE_GETOPT_LONG
> char* data_only_option = _("-f, --file=FILENAME     ");
> char* blobs_option = _("-b, --blobs         ");
> ... etc ...
> #else /* not HAVE_GETOPT_LONG */
> char* data_only_option = _("-f FILENAME             ");
> char* blobs_option = _("-b                 ");
> ... etc ...
> #endif /* not HAVE_GETOPT_LONG */
> 
> printf(_("  %s output file name\n"), data_only_option);
> printf(_("  %s include large objects in dump\n"), blobs_option);
> ... etc ...

It loos like better than the current setup.
Either way, something has to be donw about this...

-s


Thu, 2 Jan 2003 01:44:21 -0500
"Serguei Mokhov" <mokhov@cs.concordia.ca>
Re: pg_dump.options.diff 
On Thu, 2 Jan 2003 01:44:21 -0500, "Serguei Mokhov" <mokhov@cs.concordia.ca> wrote:
>Either way, something has to be donw about this...

Just another way to do it:

#if defined(HAVE_GETOPT_LONG)
#define PARMPREFIX '='
#else
#define PARMPREFIX ' '
#endif

static void
explain_option(char *shortform, char *longform, char *parm, char *desc)
{
	int pos = 0;

	printf("  -%s", shortform);
	pos += 3 + strlen(shortform);

#if defined(HAVE_GETOPT_LONG)
	printf(", --%s", longform);
	pos += 4 + strlen(longform);
#endif

	if (parm) {
		printf("%c%s", PARMPREFIX, parm);
		pos += 1 + strlen(parm);
	}/*if*/

	printf("%*c", 27 - pos, ' ');
	printf("%s\n", desc);
}/*explain_option*/

#define xo explain_option
	xo("f", "file", "FILENAME", "output file name");
	xo("F", "format", "c|t|p", "output file format (custom, tar, plain text)");
	xo("i", "ignore-version", NULL, "proceed even when server version mismatches\n"
	     "                           pg_dump version");
	xo("v", "verbose", NULL, "verbose mode");
	xo("Z", "compress", "0-9", "compression level for compressed formats");

This is only a quick hack, I didn't care for _() and
explain_option() could be smarter about multi line descriptions,
but you get the idea ...

Servus
 Manfred

Thu, 02 Jan 2003 11:52:32 +0100
Manfred Koizar <mkoi-pg@aon.at>
Re: pg_dump.options.diff 
Hi all,

I have come across some weird behavior in postgres concerning autocommit=off
and setting the transaction isolation level. I have no explanation why
things should work as they do, so I consider this a bug, no?

With autocommit=on and normal begin; ... commit; block setting the
transaction isolation level works fine:

billing=# begin;
BEGIN
billing=# set transaction isolation level serializable;
SET
billing=# show transaction isolation level;
 TRANSACTION ISOLATION LEVEL
-----------------------------
 SERIALIZABLE
(1 row)

billing=# commit;
COMMIT


Now setting autocommit=off the set transaction isolation level command does
not show any effect:

billing=# set autocommit to off;
SET
billing=# set transaction isolation level serializable;
SET
billing=# select current_date;
    date
------------
 2003-01-02
(1 row)

billing=# show transaction isolation level;
 TRANSACTION ISOLATION LEVEL
-----------------------------
 READ COMMITTED    <---- this should be SERIALIZABLE, no??
(1 row)

billing=# commit;
COMMIT

Is it a bug?
Regards,
Michael Paesold


Thu, 2 Jan 2003 13:29:07 +0100
"Michael Paesold" <mpaesold@gmx.at>
Autocommit off and transaction isolation level
Manfred Koizar <mkoi-pg@aon.at> writes:
> Just another way to do it:

> #define xo explain_option
> 	xo("f", "file", "FILENAME", "output file name");

Perhaps better would be:

#if defined(HAVE_GETOPT_LONG)
#define xo(long,short,desc)  printf("%-27s %s\n", long, desc)
#else
#define xo(long,short,desc)  printf("%-27s %s\n", short, desc)
#endif

	xo(_("-f, --file=FILENAME"),
	   _("-f FILENAME"),
	   _("output file name"));

which avoids putting a lot of "smarts" (read: restrictions) into the
subroutine, but still keeps most of the other benefits, including:
* keeping the segments of the description together in the source code,
  and rationally organized from a translation standpoint;
* only one place to change to adjust the column width.

Although it occurs to me that with the existing setup, it's possible
for the translator to unilaterally alter the column width for the
switches, which is something he'd definitely like to be able to do.
Maybe we should not try to be cute, but just do

#if defined(HAVE_GETOPT_LONG)
#define xo(long,short,desc)  printf("%s %s\n", long, desc)
#else
#define xo(long,short,desc)  printf("%s %s\n", short, desc)
#endif

	xo(_("-f, --file=FILENAME    "),
	   _("-f FILENAME            "),
	   _("output file name"));

so that the column spacing remains under control of the translator.

			regards, tom lane

Thu, 02 Jan 2003 09:29:39 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump.options.diff 
"Michael Paesold" <mpaesold@gmx.at> writes:
> Now setting autocommit=off the set transaction isolation level command does
> not show any effect:

> billing=# set autocommit to off;
> SET
> billing=# set transaction isolation level serializable;
> SET

SET does not start a transaction block, so this will not work.  You must
use an explicit BEGIN before setting TRANSACTION ISOLATION LEVEL.

You might instead set default_transaction_isolation to get the behavior
I think you are looking for.

			regards, tom lane

Thu, 02 Jan 2003 09:33:20 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Autocommit off and transaction isolation level 

On Thu, 2 Jan 2003, Tom Lane wrote:

> "Michael Paesold" <mpaesold@gmx.at> writes:
> > Now setting autocommit=off the set transaction isolation level command does
> > not show any effect:
>
> > billing=# set autocommit to off;
> > SET
> > billing=# set transaction isolation level serializable;
> > SET
>
> SET does not start a transaction block, so this will not work.  You must
> use an explicit BEGIN before setting TRANSACTION ISOLATION LEVEL.
>
> You might instead set default_transaction_isolation to get the behavior
> I think you are looking for.

The overall behavior appears to be against spec, but I figure this was
discussed at the time the set transation was added.



Thu, 2 Jan 2003 07:39:02 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: Autocommit off and transaction isolation level 
----- Original Message ----- 
From: "Tom Lane" <tgl@sss.pgh.pa.us>
Sent: January 02, 2003 9:29 AM

> Maybe we should not try to be cute, but just do
> 
> #if defined(HAVE_GETOPT_LONG)
> #define xo(long,short,desc)  printf("%s %s\n", long, desc)
> #else
> #define xo(long,short,desc)  printf("%s %s\n", short, desc)
> #endif
> 
> xo(_("-f, --file=FILENAME    "),
>    _("-f FILENAME            "),
>    _("output file name"));
> 
> so that the column spacing remains under control of the translator.

Looks good to me, but there is still a little inconvenience
of multiline option descriptions, and the above won't handle
it nicely.

If people agree with the above, can I go ahead and make corresponding
changes?

OR

may be a whole generic option-formatting routine
should be created; one for all the tools? ;-)
Similar to explain_option() of Manfred,
which will handle the mulitline, padding, and other stuff?
(am being half serious here, but it could be an "option")

-s

Thu, 2 Jan 2003 12:41:21 -0500
"Serguei Mokhov" <mokhov@cs.concordia.ca>
Re: pg_dump.options.diff 
"Serguei Mokhov" <mokhov@cs.concordia.ca> writes:
> Looks good to me, but there is still a little inconvenience
> of multiline option descriptions, and the above won't handle
> it nicely.

True, a multiline description would have to look like

	xo(_("-f, --file=FILENAME    "),
	   _("-f FILENAME            "),
	   _("output file name\n"
	     "	                     more description"));

Which is not great, but it doesn't seem completely unworkable either.
And the translator can still adjust the column spacing without any
code changes.

> may be a whole generic option-formatting routine
> should be created; one for all the tools? ;-)
> Similar to explain_option() of Manfred,
> which will handle the mulitline, padding, and other stuff?
> (am being half serious here, but it could be an "option")

The trouble I see there is that the layout --- in particular the column
width --- would be embedded in such a routine and not alterable by
simply replacing message texts.

			regards, tom lane

Thu, 02 Jan 2003 13:58:27 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump.options.diff 
----- Original Message ----- 
From: "Tom Lane" <tgl@sss.pgh.pa.us>
Sent: January 02, 2003 1:58 PM

> "Serguei Mokhov" <mokhov@cs.concordia.ca> writes:
> > Looks good to me, but there is still a little inconvenience
> > of multiline option descriptions, and the above won't handle
> > it nicely.
> 
> True, a multiline description would have to look like
> 
> xo(_("-f, --file=FILENAME    "),
>    _("-f FILENAME            "),
>    _("output file name\n"
>      "                      more description"));
> 
> Which is not great, but it doesn't seem completely unworkable either.
> And the translator can still adjust the column spacing without any
> code changes.

Well, it's better than before and solves *my* (and other translators')
problem. 

Now, this:

#if defined(HAVE_GETOPT_LONG)
#define xo(long,short,desc)  printf("%s %s\n", long, desc)
#else
#define xo(long,short,desc)  printf("%s %s\n", short, desc)
#endif

seems relatively generic, so it could be used by more than one tool.

I searched for 'util' the source tree to see a more or less
logical place to put it. I got a whole bunch of .*util.* files,
but none of them seems appropriate enough because they all specific
to some other tool or smth else. Is pushing it up to c.h an option,
or it'll become too polluted? Where should I place it?

> > may be a whole generic option-formatting routine
> > should be created; one for all the tools? ;-)
> > Similar to explain_option() of Manfred,
> > which will handle the mulitline, padding, and other stuff?
> > (am being half serious here, but it could be an "option")
> 
> The trouble I see there is that the layout --- in particular the column
> width --- would be embedded in such a routine and not alterable by
> simply replacing message texts.

True, but what would be wrong by having an argument for the column width?

-s

Thu, 2 Jan 2003 15:04:12 -0500
"Serguei Mokhov" <mokhov@cs.concordia.ca>
Re: pg_dump.options.diff 
"Serguei Mokhov" <mokhov@cs.concordia.ca> writes:
> Now, this:

> #if defined(HAVE_GETOPT_LONG)
> #define xo(long,short,desc)  printf("%s %s\n", long, desc)
> #else
> #define xo(long,short,desc)  printf("%s %s\n", short, desc)
> #endif

> seems relatively generic, so it could be used by more than one tool.

But there's no good place to put it.  I'd say just stick it into each
tool; it's no worse than repeating the existence of a "usage()"
subroutine in each tool.

> Is pushing it up to c.h an option,

I'd vote against that.

>> The trouble I see there is that the layout --- in particular the column
>> width --- would be embedded in such a routine and not alterable by
>> simply replacing message texts.

> True, but what would be wrong by having an argument for the column width?

The translator would have no control over such an argument --- at least
not without some mechanism outside the .po files.

			regards, tom lane

Thu, 02 Jan 2003 15:20:35 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump.options.diff 
----- Original Message ----- 
From: "Tom Lane" <tgl@sss.pgh.pa.us>
Sent: January 02, 2003 3:20 PM

> > #if defined(HAVE_GETOPT_LONG)
> > #define xo(long,short,desc)  printf("%s %s\n", long, desc)
> > #else
> > #define xo(long,short,desc)  printf("%s %s\n", short, desc)
> > #endif
> 
> > seems relatively generic, so it could be used by more than one tool.
> 
> But there's no good place to put it.  I'd say just stick it into each
> tool; it's no worse than repeating the existence of a "usage()"
> subroutine in each tool.

It ended up being in dumputils.h

Attached a patch for pg_dump(all) and pg_restore eliminating dupes
in option descriptions as per above xo "technology".

Please review and apply if there are no too many objections.

thank you,
-s
Thu, 2 Jan 2003 17:51:55 -0500
"Serguei Mokhov" <mokhov@cs.concordia.ca>
Re: pg_dump.options.diff 
"Serguei Mokhov" <mokhov@cs.concordia.ca> writes:
>> But there's no good place to put it.  I'd say just stick it into each
>> tool; it's no worse than repeating the existence of a "usage()"
>> subroutine in each tool.

> It ended up being in dumputils.h

I really don't like putting a macro with a name as short as "xo" into a
header file, even one of relatively narrow scope.  It's too likely to
create weird conflicts.  My inclination is to make the coding be more
like

static void
usage(void)
{
#if defined(HAVE_GETOPT_LONG)
#define xo(longOption,shortOption,desc)  printf("%s %s\n", longOption, desc)
#else
#define xo(longOption,shortOption,desc)  printf("%s %s\n", shortOption, desc)
#endif

	... lots of xo() calls ...

#undef xo
}

This gives us the convenience of a very short name within the usage()
subroutines, while not polluting the namespace for everyplace else in
these utilities.

As I said before, duplicating the definition of xo() in each file that
uses it doesn't bother me a bit; it's too simple for that to be a
significant objection.

			regards, tom lane

Thu, 02 Jan 2003 18:16:07 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump.options.diff 
Serguei Mokhov writes:

> #if defined(HAVE_GETOPT_LONG)
> #define xo(long,short,desc)  printf("%s %s\n", long, desc)
> #else
> #define xo(long,short,desc)  printf("%s %s\n", short, desc)
> #endif
>
> seems relatively generic, so it could be used by more than one tool.

As long as we're spending time on this, why not just write our own version
of getopt_long()?

-- 
Peter Eisentraut   peter_e@gmx.net


Sat, 4 Jan 2003 11:29:40 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: pg_dump.options.diff 
Peter Eisentraut <peter_e@gmx.net> writes:
> As long as we're spending time on this, why not just write our own version
> of getopt_long()?

Seems like a fine idea to me ... who's volunteering?

			regards, tom lane

Sat, 04 Jan 2003 11:02:01 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump.options.diff 
Tom Lane writes:

> > As long as we're spending time on this, why not just write our own version
> > of getopt_long()?
>
> Seems like a fine idea to me ... who's volunteering?

Doing it now...

-- 
Peter Eisentraut   peter_e@gmx.net


Sun, 5 Jan 2003 00:26:47 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: pg_dump.options.diff 