Thread: Order getopt arguments
I had noticed that most getopt() or getopt_long() calls had their letter lists in pretty crazy orders. There might have been occasional attempts at grouping, but those then haven't been maintained as new options were added. To restore some sanity to this, I went through and ordered them alphabetically.
Attachment
Hello Peter, > I had noticed that most getopt() or getopt_long() calls had their letter > lists in pretty crazy orders. There might have been occasional attempts > at grouping, but those then haven't been maintained as new options were > added. To restore some sanity to this, I went through and ordered them > alphabetically. I agree that a more or less random historical order does not make much sense. For pgbench, ISTM that sorting per functionality then alphabetical would be better than pure alphabetical because it has 2 modes. Such sections might be (1) general (2) connection (3) common/shared (4) initialization and (5) benchmarking, we some comments on each. What do you think? If okay, I'll send you a patch for that. -- Fabien.
On Mon, Dec 5, 2022 at 3:42 AM Fabien COELHO <coelho@cri.ensmp.fr> wrote: > > I had noticed that most getopt() or getopt_long() calls had their letter > > lists in pretty crazy orders. There might have been occasional attempts > > at grouping, but those then haven't been maintained as new options were > > added. To restore some sanity to this, I went through and ordered them > > alphabetically. > > I agree that a more or less random historical order does not make much > sense. > > For pgbench, ISTM that sorting per functionality then alphabetical would > be better than pure alphabetical because it has 2 modes. Such sections > might be (1) general (2) connection (3) common/shared (4) initialization > and (5) benchmarking, we some comments on each. I don't see the value in this. Grouping related options often makes sense, but it seems more confusing than helpful in the case of a getopt string. +1 for Peter's proposal to just alphabetize. That's easy to maintain, at least in theory. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes: > +1 for Peter's proposal to just alphabetize. That's easy to maintain, > at least in theory. Agreed for single-letter options. Long options complicate matters: are we going to order their code stanzas by the actual long name, or by the character/number returned by getopt? Or are we going to be willing to repeatedly renumber the assigned codes to keep those the same? I don't think I want to go that far. regards, tom lane
On Mon, Dec 5, 2022 at 11:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > +1 for Peter's proposal to just alphabetize. That's easy to maintain, > > at least in theory. > > Agreed for single-letter options. Long options complicate matters: > are we going to order their code stanzas by the actual long name, or > by the character/number returned by getopt? Or are we going to be > willing to repeatedly renumber the assigned codes to keep those the > same? I don't think I want to go that far. I was only talking about the actual argument to getopt(), not the order of the code stanzas. I'm not sure what we ought to do about the latter. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes: > I was only talking about the actual argument to getopt(), not the > order of the code stanzas. I'm not sure what we ought to do about the > latter. 100% agreed that the getopt argument should just be alphabetical. But the bulk of Peter's patch is rearranging switch cases to agree with that, and if you want to do that then you have to also think about long options, which are not in the getopt argument. I'm not entirely convinced that reordering the switch cases is worth troubling over. regards, tom lane
On Mon, Dec 5, 2022 at 11:51 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > I was only talking about the actual argument to getopt(), not the > > order of the code stanzas. I'm not sure what we ought to do about the > > latter. > > 100% agreed that the getopt argument should just be alphabetical. > But the bulk of Peter's patch is rearranging switch cases to agree > with that, and if you want to do that then you have to also think > about long options, which are not in the getopt argument. I'm > not entirely convinced that reordering the switch cases is worth > troubling over. I'm not particularly sold on that either, but neither am I particularly opposed to it. -- Robert Haas EDB: http://www.enterprisedb.com
On 05.12.22 18:04, Robert Haas wrote: > On Mon, Dec 5, 2022 at 11:51 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >>> I was only talking about the actual argument to getopt(), not the >>> order of the code stanzas. I'm not sure what we ought to do about the >>> latter. >> >> 100% agreed that the getopt argument should just be alphabetical. >> But the bulk of Peter's patch is rearranging switch cases to agree >> with that, and if you want to do that then you have to also think >> about long options, which are not in the getopt argument. I'm >> not entirely convinced that reordering the switch cases is worth >> troubling over. > > I'm not particularly sold on that either, but neither am I > particularly opposed to it. I have committed it as posted.