On Wed, Jul 28, 2021 at 09:28:17AM +0200, Pavel Stehule wrote:
> Here is an updated implementation of filter's file, that implements syntax
> proposed by you.
Thanks.
If there's any traction for this approach. I have some comments for the next
revision,
> +++ b/doc/src/sgml/ref/pg_dump.sgml
> @@ -789,6 +789,56 @@ PostgreSQL documentation
> </listitem>
> </varlistentry>
>
> + <varlistentry>
> + <term><option>--filter=<replaceable
> class="parameter">filename</replaceable></option></term>
> + <listitem>
> + <para>
> + Read objects filters from the specified file.
> + If you use "-" as a filename, the filters are read from stdin.
Say 'Specify "-" to read from stdin'
> + The lines starting with symbol <literal>#</literal> are ignored.
Remove "The" and "symbol"
> + Previous white chars (spaces, tabs) are not allowed. These
Preceding whitespace characters...
But actually, they are allowed? But if it needs to be explained, maybe they
shouldn't be - I don't see the utility of it.
> +static bool
> +isblank_line(const char *line)
> +{
> + while (*line)
> + {
> + if (!isblank(*line++))
> + return false;
> + }
> +
> + return true;
> +}
I don't think this requires nor justifies having a separate function.
Either don't support blank lines, or use get_keyword() with size==0 for that ?
> + /* Now we expect sequence of two keywords */
> + if (keyword && is_keyword(keyword, size, "include"))
> + is_include = true;
> + else if (keyword && is_keyword(keyword, size, "exclude"))
> + is_include = false;
> + else
I think this should first check "if keyword == NULL".
That could give a more specific error message like "no keyword found",
> + exit_invalid_filter_format(fp,
> +
> filename,
> +
> "expected keyword \"include\" or \"exclude\"",
> +
> line.data,
> +
> lineno);
..and then this one can say "invalid keyword".
--
Justin
