On 2018/08/17 12:42, Tatsuro Yamada wrote:
On 2018/08/17 11:47, Michael Paquier wrote:
On Thu, Aug 16, 2018 at 08:57:57PM +0900, Michael Paquier wrote:
I agree on both points. Any objections if I apply what's proposed here
on HEAD?
I have been looking at this patch. And while consistency is nice, I
think that if we are cleaning up this stuff we could do a bit more to
be more consistent with the other binary tools:
- Addition of long options for connection options.
- removal of -h, and use it for host value instead of "-H".
- Use "USERNAME" instead of "NAME" in help().
vacuumlo could also be improved a bit...
Yeah, I'll revise the patch based on your suggestions.
I created WIP patch for oid2name and vacuumlo includes followings:
oid2name and vacuumlo
- Add long options
only oid2name
- Replace -H with -h
- Replace NAME with USERNAME
I'll revise their documents and create new patch next week, probably. :)
Regards,
Tatsuro Yamada
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index 63e360c4c5..3501a9c2cc 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -60,8 +60,25 @@ void sql_exec_dumpalltbspc(PGconn *, struct options *);
void
get_opts(int argc, char **argv, struct options *my_opts)
{
+ static struct option long_options[] = {
+ {"host", required_argument, NULL, 'h'},
+ {"port", required_argument, NULL, 'p'},
+ {"username", required_argument, NULL, 'U'},
+ {"dbname", required_argument, NULL, 'd'},
+ {"tablename", required_argument, NULL, 't'},
+ {"oid", required_argument, NULL, 'o'},
+ {"filenode", no_argument, NULL, 'f'},
+ {"quiet", no_argument, NULL, 'q'},
+ {"system", no_argument, NULL, 'S'},
+ {"extend", no_argument, NULL, 'x'},
+ {"index", no_argument, NULL, 'i'},
+ {"tablespace", no_argument, NULL, 's'},
+ {NULL, 0, NULL, 0}
+ };
+
int c;
const char *progname;
+ int optindex;
progname = get_progname(argv[0]);
@@ -93,10 +110,25 @@ get_opts(int argc, char **argv, struct options *my_opts)
}
/* get opts */
- while ((c = getopt(argc, argv, "H:p:U:d:t:o:f:qSxish")) != -1)
+ while ((c = getopt_long(argc, argv, "h:p:U:d:t:o:f:qSxis", long_options, &optindex)) != -1)
{
switch (c)
{
+ /* host to connect to */
+ case 'h':
+ my_opts->hostname = pg_strdup(optarg);
+ break;
+
+ /* port to connect to on remote host */
+ case 'p':
+ my_opts->port = pg_strdup(optarg);
+ break;
+
+ /* username */
+ case 'U':
+ my_opts->username = pg_strdup(optarg);
+ break;
+
/* specify the database */
case 'd':
my_opts->dbname = pg_strdup(optarg);
@@ -122,46 +154,26 @@ get_opts(int argc, char **argv, struct options *my_opts)
my_opts->quiet = true;
break;
- /* host to connect to */
- case 'H':
- my_opts->hostname = pg_strdup(optarg);
- break;
-
- /* port to connect to on remote host */
- case 'p':
- my_opts->port = pg_strdup(optarg);
- break;
-
- /* username */
- case 'U':
- my_opts->username = pg_strdup(optarg);
- break;
-
/* display system tables */
case 'S':
my_opts->systables = true;
break;
- /* also display indexes */
- case 'i':
- my_opts->indexes = true;
- break;
-
/* display extra columns */
case 'x':
my_opts->extended = true;
break;
+ /* also display indexes */
+ case 'i':
+ my_opts->indexes = true;
+ break;
+
/* dump tablespaces only */
case 's':
my_opts->tablespaces = true;
break;
- case 'h':
- help(progname);
- exit(0);
- break;
-
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -176,20 +188,21 @@ help(const char *progname)
"Usage:\n"
" %s [OPTION]...\n"
"\nOptions:\n"
- " -d DBNAME database to connect to\n"
- " -f FILENODE show info for table with given file node\n"
- " -H HOSTNAME database server host or socket directory\n"
- " -i show indexes and sequences too\n"
- " -o OID show info for table with given OID\n"
- " -p PORT database server port number\n"
- " -q quiet (don't show headers)\n"
- " -s show all tablespaces\n"
- " -S show system objects too\n"
- " -t TABLE show info for named table\n"
- " -U NAME connect as specified database user\n"
- " -V, --version output version information, then exit\n"
- " -x extended (show additional columns)\n"
- " -?, --help show this help, then exit\n"
+ " -f, --filenode FILENODE show info for table with given file node\n"
+ " -i, --index show indexes and sequences too\n"
+ " -o, --oid=OID show info for table with given OID\n"
+ " -q, --quiet quiet (don't show headers)\n"
+ " -s, --tablespace show all tablespaces\n"
+ " -S, --system show system objects too\n"
+ " -t, --table=TABLE show info for named table\n"
+ " -V, --version output version information, then exit\n"
+ " -x, --extend extended (show additional columns)\n"
+ " -?, --help show this help, then exit\n"
+ "\nConnection options:\n"
+ " -d, --dbname=DBNAME database to connect to\n"
+ " -h, --host=HOSTNAME database server host or socket directory\n"
+ " -p, --port=PORT database server port number\n"
+ " -U, --username=USERNAME connect as specified database user\n"
"\nThe default action is to show all database OIDs.\n\n"
"Report bugs to <pgsql-b...@postgresql.org>.\n",
progname, progname);
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index 7eb474ca3e..321ce8ae8c 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -434,17 +434,17 @@ usage(const char *progname)
printf("%s removes unreferenced large objects from databases.\n\n", progname);
printf("Usage:\n %s [OPTION]... DBNAME...\n\n", progname);
printf("Options:\n");
- printf(" -l LIMIT commit after removing each LIMIT large objects\n");
- printf(" -n don't remove large objects, just show what would be done\n");
- printf(" -v write a lot of progress messages\n");
- printf(" -V, --version output version information, then exit\n");
- printf(" -?, --help show this help, then exit\n");
+ printf(" -l, --limit=LIMIT commit after removing each LIMIT large objects\n");
+ printf(" -n, --dry-run don't remove large objects, just show what would be done\n");
+ printf(" -v, --verbose write a lot of progress messages\n");
+ printf(" -V, --version output version information, then exit\n");
+ printf(" -?, --help show this help, then exit\n");
printf("\nConnection options:\n");
- printf(" -h HOSTNAME database server host or socket directory\n");
- printf(" -p PORT database server port\n");
- printf(" -U USERNAME user name to connect as\n");
- printf(" -w never prompt for password\n");
- printf(" -W force password prompt\n");
+ printf(" -h, --host=HOSTNAME database server host or socket directory\n");
+ printf(" -p, --port=PORT database server port\n");
+ printf(" -U, --username=USERNAME user name to connect as\n");
+ printf(" -w, --no-password never prompt for password\n");
+ printf(" -W, --password force password prompt\n");
printf("\n");
printf("Report bugs to <pgsql-b...@postgresql.org>.\n");
}
@@ -453,11 +453,24 @@ usage(const char *progname)
int
main(int argc, char **argv)
{
+ static struct option long_options[] = {
+ {"host", required_argument, NULL, 'h'},
+ {"limit", required_argument, NULL, 'l'},
+ {"username", required_argument, NULL, 'U'},
+ {"port", required_argument, NULL, 'p'},
+ {"verbose", no_argument, NULL, 'v'},
+ {"dry-run", no_argument, NULL, 'n'},
+ {"no-password", no_argument, NULL, 'w'},
+ {"password", no_argument, NULL, 'W'},
+ {NULL, 0, NULL, 0}
+ };
+
int rc = 0;
struct _param param;
int c;
int port;
const char *progname;
+ int optindex;
progname = get_progname(argv[0]);
@@ -486,12 +499,8 @@ main(int argc, char **argv)
}
}
- while (1)
+ while((c = getopt_long(argc, argv, "h:l:U:p:vnwW", long_options, &optindex)) != -1)
{
- c = getopt(argc, argv, "h:l:U:p:vnwW");
- if (c == -1)
- break;
-
switch (c)
{
case '?':
