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.