Thread
-
Re: Generate GUC tables from .dat file
David E. Wheeler <david@justatheory.com> — 2025-08-28T18:03:32Z
On Aug 28, 2025, at 09:29, Daniel Gustafsson <daniel@yesql.se> wrote: > A tiny nitpick is that all the other generator scripts (that I looked at) in > the tree use GetOptions() with named parameter rather than dereference ARGV > directly: > > +my $input_fname = $ARGV[0]; > +my $output_fname = $ARGV[1]; GetOptions() is overkill when there are no options. I’d opt for: die "Usage: $0 INPUT_FILE OUTPUT_FILE\n" unless @ARGV == 2; my ($input_fname, $output_fname) = @ARGV; And since I griped about Perl style previously, I made a pass over modernizing it a bit. One might argue it’s less clear, of course; there is less alignment of the printing than in the original. Otherwise, I’d note: * Use the /r regex return sequence to simplify dquote() (requires Perl 5.14, IIRC) * Iterate over the data types in a single line * Pass lists of values to `print` * Use {$fh} syntax to make file handle arguments clearer But do with it what you will. One other thing, as an aside, and probably not worth changing this patch: I’d prefer to see the use of explicit I/O layers. IOW, rather than open my $ofh, '>', $output_fname Use the UTF-8 layer, which encodes strings as UTF-8 bytes: open my $ofh, '>:utf8', $output_fname Or perhaps use pure binary: open my $ofh, '>:raw', $output_fname Though then things like `ucfirst` won’t work properly for non-ASCII strings. The default layer, when not specified, is Latin-1 (because 1994). It’s not a problem if we’re certain we’ll never use anything other than ASCII, but more explicit I/O layers would be clearer, IMO. I didn’t change it in the attached because Catalog.pm doesn’t use an I/O layer, either, so it’s best if they’re consistent. So, I guess, would there be interest in a patch to update I/O layer handling in the core Perl code? Best, David