po 13. 9. 2021 v 15:11 odesÃlatel Justin Pryzby <pry...@telsasoft.com>
napsal:
> 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".
>
I fixed (I hope) mentioned issues. Please check last patch
Regards
Pavel
> --
> Justin
>
