On Wed, Jan 22, 2025 at 03:34:11PM +0000, Dagfinn Ilmari Mannsåker wrote: > As I mentioned elsewhere in the thread, I left those alone since the > error message uses the short spelling even if the command had the long > one. We could add the long spelling to the preceding comments, like the > second attached patch.
For this one, I am wondering one thing. Wouldn't it be more user-friendly if we used the same convention as some of the other tools, mentioning options in the shape of "-N/--option", like pg_dump? It does not feel quite right to me to report an error only for the short option when using the long option name. > Most invocations of pg_ctl actually have the action before any options: > > ❯ rg --multiline -tperl '(\W)pg_ctl\1,\s*(\W)\w+\2' | rg -cw pg_ctl > 23 > > ❯ rg --multiline -tperl '(\W)pg_ctl\1,\s*(\W)--\w+\2' | rg -cw pg_ctl > 11 > > I've attached a third patch to make them consistently have the action > first. Not sure that this is worth bothering. Originally, the reason why the modes of pg_ctl were kept last in these commands was because of our getopt() implementations in src/port/ where WIN32 did not like the modes when they were not last. It does not matter these days, so keeping these as they are is equally OK. Thanks for the pg_amcheck part. I've noticed three more that depend on non-existing objects in src/bin/scripts/, so I have grouped them while on it, and applied the result. While looking at non-existing patterns in the .pl files, I've bumped into pg_basebackup's 010 having a few more things going on, as well.. -- Michael
signature.asc
Description: PGP signature
