272623 - FindWrapPoint is misplaced in process_bug.cgi

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Christian Reis]

The function FindWrapPoint is misplaced in process_bug.cgi -- LogActivityEntry
in CGI.pl requires it.

Comment 1

[Christian Reis]

Attachment: kiko_v1: obvious

Comment 2

[Dave Miller [:justdave]]

Comment on attachment 167567 [details] [diff] [review]
kiko_v1: obvious

hmmm...  yeah, I guess so.  I hate to add things to CGI.pl, since we're trying
to get rid of it, but I can't think of a more appropriate place at the moment. 
Perhaps Bug.pm or Bug/Activity.pm or something similar that doesn't exist yet.
:)

Comment 3

[Max Kanat-Alexander]

Comment on attachment 167567 [details] [diff] [review]
kiko_v1: obvious

Put it in Bugzilla::Util, now that we have that. :-)

Comment 4

[Frédéric Buclin]

Attachment: patch, v2

Move FindWrapPoint to Util.pm and rename it to find_wrap_point.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 180876 [details] [diff] [review]
patch, v2

Looks good, but a few points:

The POD docs should specify that it actually starts searching at the end of the
string, for a position to wrap from.

And from the caller's perspective, I think $startpos is a confusing variable
name, because it sounds like where we are starting a *forward* search of the
string. Maybe $line_length or something like that, instead?

Finally, that 254 in CGI.pl should become a constant instead of a raw number,
while we're hitting that line anyway.

Comment 6

[Frédéric Buclin]

Attachment: patch, v3

fix all mkanat' comments.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 180928 [details] [diff] [review]
patch, v3

OK, but MAX_LINE_LENGTH needs a comment explaining what it is and where it's
used, and the POD docs need to explain what the function returns. (Right now,
it sounds like it returns a split string, which isn't true.)

Comment 8

[Frédéric Buclin]

POD docs in Util.pm are already there. And I won't add anything more than a
small comment in CGI.pl as this file will soon disappear. I will add the comment
on checkin.

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> POD docs in Util.pm are already there. And I won't add anything more than a
> small comment in CGI.pl as this file will soon disappear. I will add the comment
> on checkin.

  Yeah, it's going away, but when we move the constant the comment will be moved
with it.

Comment 10

[Frédéric Buclin]

Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.240; previous revision: 1.239
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.251; previous revision: 1.250
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.24; previous revision: 1.23
done