On Tue, 10 Nov 2020 at 03:07, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2020-Nov-06, David Rowley wrote:
>
> > +# Handle makefile rules for when file to be added to the project
> > +# does not exist. Returns 1 when the original file add should be
> > +# skipped.
> > +sub FindAndAddAdditionalFiles
> > +{
> > + my $self = shift;
> > + my $fname = shift;
> > + my ($ext) = $fname =~ /(\.[^.]+)$/;
> > +
> > + # For .c files, check if a .l file of the same name exists and add that
> > + # too.
> > + if ($ext eq ".c")
> > + {
> > + my $filenoext = $fname;
> > + $filenoext =~ s{\.[^.]+$}{};
>
> I think you can make this simpler by capturing both the basename and the
> extension in one go. For example,
>
> $fname =~ /(.*)(\.[^.]+)$/;
> $filenoext = $1;
> $ext = $2;
>
> so you avoid the second =~ statement.
Thanks. That's neater.
> > + if (-e "$filenoext.l")
> > + {
> > + AddFileConditional($self, "$filenoext.l");
> > + return 1;
> > + }
> > + if (-e "$filenoext.y")
> > + {
> > + AddFileConditional($self, "$filenoext.y");
>
> Maybe DRY like
>
> for my $ext (".l", ".y") {
> my $file = $filenoext . $ext;
> AddFileConditional($self, $file) if -f $file;
> return 1;
> }
I did adapt that part of the code, but not exactly to what's above.
The return there would cause us to return from the function after the
first iteration.
> Note: comment says "check if a .l file" and then checks both .l and .y.
> Probably want to update the comment ...
Updated.
I've attached the v3 patch.
I'm still working through some small differences in some of the
.vcxproj files. I've been comparing these by copying *.vcxproj out to
another directory with patched and unpatched then diffing the
directory. See attached txt file with those diffs. Here's a summary of
some of them:
1. There are a few places that libpq gets linked where it previously did not.
2. REFINT_VERBOSE gets defined in a few more places than it did
previously. This makes it closer to what happens on Linux anyway, if
you look at the Make output from contrib/spi/Makefile you'll see
-DREFINT_VERBOSE in there for autoinc
3. LOWER_NODE gets defined in ltree now where it wasn't before. It's
defined on Linux. Unsure why it wasn't before on Windows.
David