Jussi Sirpoma Reporter
	Description • 20 years ago

Bug 11901 improved bugzilla's output formatting substantially, but in the
process it lost the ability to insert peaces of code or other preformatted text
to comments. The below link shows a peace of bugzilla code copied to a comment:

<http://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=2383#c2>

To fix this, I would suggest adding a tag which could be used to turn off
wrapping when necessary. For example:

<verbatim>
    Preformatted text.
</verbatim>

	Max Kanat-Alexander Assignee
	Comment 1 • 20 years ago

(In reply to comment #0)
> Bug 11901 improved bugzilla's output formatting substantially, but in the
> process it lost the ability to insert peaces of code or other preformatted text
> to comments. 

  Nothing was lost. Bugzilla never had that ability, unless you hacked it.

  You can also now add a > to the front of a line to make it not wrap.

	Jussi Sirpoma Reporter
	Comment 2 • 20 years ago

(In reply to comment #1)
> Nothing was lost. Bugzilla never had that ability, unless you hacked it.
> You can also now add a > to the front of a line to make it not wrap.

You are right, but before output formatting bugzilla had kind of WYSIWYG comment
writing where everything was preformatted. So I was able to insert indented code
etc. without loosing readability.  See the link in comment 0 which contains a
peace of code copied from User.pm. If I would copy the same code here, it would
show up indented correctly.

	Max Kanat-Alexander Assignee
	Comment 3 • 20 years ago

Oh, that's a bug! :-)

	Max Kanat-Alexander Assignee
	Comment 4 • 20 years ago

OK, so basically, I discovered that this behavior of Text::Autoformat is not
changeable.

So I went back to how we used to wrap all attachment comments, but put that
code into the wrap_comment sub.

So this means that we're back to using Text::Wrap again, which I really like
more anyhow, and which I think is probably also faster.

	Max Kanat-Alexander Assignee
	Comment 5 • 20 years ago

This pretty much definitely blocks any release.

Here's a testing installation, and a link to a bug where I've posted many
test-case comments:

http://landfill.bugzilla.org/bz282349/show_bug.cgi?id=2323

I didn't want to make this a "back out bug 11901 and check it back in" because
this only affects a localized part of that patch.

	Myk Melez [:myk] [@mykmelez]
	Comment 6 • 20 years ago

Comment on attachment 174473 [details] [diff] [review]
Go back to using Text::Wrap, with same functionality

This seems ok if it doesn't regress functionality, but it should get second
review from someone who reviewed the fix for bug 11901 and is familiar with the
decision to go with Text::Autoformat in that patch.

	Max Kanat-Alexander Assignee
	Comment 7 • 20 years ago

Comment on attachment 174473 [details] [diff] [review]
Go back to using Text::Wrap, with same functionality

OK, that would be Gerv.

Gerv, could you respond to Myk's comment above?

	Gervase Markham [:gerv]
	Comment 8 • 20 years ago

The reason we chose Text::Autoformat was issue 2 in bug 11901, comment 229.
However, you appear to have fixed that with WRAP_IGNORE in your patch (which,
BTW, should be hard coded rather than yet another global variable; it's _so_ not
going to change.)

I find it hard to believe that AutoFormat a) does what you say it does, and b)
isn't configurable in it. But if we've solve issue #2, then I have no particular
problem with switching back to Text::Wrap.

Until, of course, we find some more comments that _that_ can't cope with...

Gerv

	Max Kanat-Alexander Assignee
	Comment 9 • 20 years ago

It does indeed do what I say it does, and it's definitely not configurable. I
pored over the Text::Autoformat source for hours.

So if I switch WRAP_IGNORE to be a literal instead of a constant, is that a r+?

	Max Kanat-Alexander Assignee
	Comment 10 • 20 years ago

Oops. Gerv wasn't on CC. Gerv -- see my last comment.

	Gervase Markham [:gerv]
	Comment 11 • 20 years ago

Comment on attachment 174473 [details] [diff] [review]
Go back to using Text::Wrap, with same functionality

r=gerv with that change. I hope there's not a performance hit associated with
calling wrap for every line rather than once per comment...

Gerv

	Max Kanat-Alexander Assignee
	Comment 12 • 20 years ago

OK, carrying forward r+ from Gerv based on the comments above.

	Max Kanat-Alexander Assignee
	Comment 13 • 20 years ago

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.356; previous revision: 1.355
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.23; previous revision: 1.22
done