Ticket #128 (closed enhancement: fixed)

Opened 6 years ago

Last modified 6 years ago

[PATCH] perl.{req,prov} cleanups


Reported by: scop Assigned to: pmatilai Priority: minor Milestone:
Component: rpm Version: RPM Development Keywords: Cc:


Description

The attached patch cleans up trailing whitespace, converts tabs to spaces, does some other whitespace cleanups, and avoids some unnecessary backslashes and parens in perl.prov and perl.req. This is just cleanup, there are no functional changes.

Attachments

0004-perl.-req-prov-whitespace-backslash-and-paren-clea.patch (9.0 kB) - added by scop on 01/24/10 11:51:36.
perl.{req,prov} cleanups
0002-More-here-doc-skipping-fixes-for-perl.req-128.patch (0.8 kB) - added by scop on 03/03/10 18:51:17.
More here-doc skipping fixes as discussed

Change History

01/24/10 11:51:36 changed by scop

  • attachment 0004-perl.-req-prov-whitespace-backslash-and-paren-clea.patch added.

perl.{req,prov} cleanups

01/24/10 11:51:54 changed by scop

  • type changed from defect to enhancement.

01/25/10 10:37:05 changed by pmatilai

  • owner changed from RpmTickets to pmatilai.
  • status changed from new to assigned.

...and applied. Thanks for the patch.

01/25/10 10:37:16 changed by pmatilai

  • status changed from assigned to closed.
  • resolution set to fixed.

03/03/10 02:09:45 changed by Anonymous

  • priority changed from trivial to minor.
  • status changed from closed to reopened.
  • resolution deleted.

There is still a remaining problem with this patch. The pattern in line 85 of perl.req will match anything starting with some kind of quote character and ending with the same quote. But that might inappropriately include some extra copies of the same quote. For instance, if the source text in the file is:

$make_frag .= <<'MAKE_FRAG' if $self->is_make_type('dmake');

then this is what will match and be treated as the tag:

MAKE_FRAG' if $self->is_make_type('dmake

which is clearly not what what was intended. The string-content pattern matching needs to be changed to be not-greedy:

m/^\s*\$(?:.*)\s*=\s*<<\s*(["'`])(.*?)\1/

03/03/10 18:49:33 changed by scop

Hm, I don't see how it would be a problem with this patch - this patch does not do any functional changes, it's just cleanup. This would be closer (but the problem is not introduced by it either): https://bugzilla.redhat.com/attachment.cgi?id=362125&action=diff

Anyway, your suggestion makes sense as a further improvement. I suppose while at it the last parenthesis should be changed from * to + because something is always wanted between the quotes, ditto on the next line for \w, no?

03/03/10 18:51:17 changed by scop

  • attachment 0002-More-here-doc-skipping-fixes-for-perl.req-128.patch added.

More here-doc skipping fixes as discussed

03/24/10 08:34:11 changed by pmatilai

  • status changed from reopened to closed.
  • resolution set to fixed.

Second patch also applied now, thanks.