Hello Daniel,
> PFA a v3 patch that implements that, along with regression tests this
> time.
My 0.02 €:
Patch applies cleanly, compiles, make check ok, doc generation ok.
I'm in favor of having a simple psql way to generate a convenient and
compliant CSV output for export/import.
I also think that a short option brings little value, and "--csv" is good
enough for the purpose, so I would agree to remove the "-C" binding.
About "fieldsep_csv", I do not like much the principle of having different
output variables to represent the same concept depending on the format. I
would rather have reused fieldsep as in your previous submission and set
it to "," when under --csv. This is not a strong opinion and other people
may differ: the committer opinion is the one to follow:-)
The "\n" eol style is hardcoded. Should it use "recordsep"? For instance,
https://tools.ietf.org/html/rfc4180 seems to specify CRLF end of lines.
The definition is evolving, eg https://www.w3.org/TR/tabular-data-model/
accepts both "\r" and "\r\n". I do not like using windows eol, but I think
that it should be possible to do it, which is not the case with this
version.
The "\pset format" error message in "do_pset" shows values in seemingly
random order. The situation is pre-existing but not really satisfactory.
I'd suggest to put all values in alphabetical order.
In csv_print_field & csv_print_text, you are not consistent when putting
braces on blocks with only one instruction. I'd suggest not to put braces
in that case.
I'd suggest that tests should include more types, not just strings. I
would suggest int, float, timestamp, bytea, an array (which uses , as a
separator), json (which uses both " and ,)...
--
Fabien.
