Last modified: 2013-10-11 18:51:06 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T41767, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 39767 - Break lines in review Gerrit comments
Break lines in review Gerrit comments
Status: VERIFIED FIXED
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: Normal trivial (vote)
: ---
Assigned To: Dereckson
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-29 16:06 UTC by Dereckson
Modified: 2013-10-11 18:51 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Dereckson 2012-08-29 16:06:22 UTC
Currently, Gerrit doesn't break lines in review comments, although we sometimes write code or commands there.

Example of such comment:
* https://gerrit.wikimedia.org/r/#/c/16606/ (my patch set 6 review comment)

It already correctly break lines (I've heard there were an issue under Opera) for inline comments:
* https://gerrit.wikimedia.org/r/#/c/18224/1/skins/common/preview.js
Comment 1 Dereckson 2012-08-29 16:07:14 UTC
Taking this bug.
Comment 2 Chad H. 2012-08-29 16:08:52 UTC
Most likely any fix for this needs to involve upstream. Either

A) The class needs to be marked @external so it doesn't change from one useless string to another on upgrade, or
B) gerrit.css needs to be fixed upstream so this is fixed for everyone

:)
Comment 3 Dereckson 2012-08-29 16:14:17 UTC
If we'd want to be wild and overkill the kludge, I tested with success (ie it doesn't break any interface element anywhere - except in documentation, which wouldn't be covered by our CSS) the following snippet in Greasemoney:

GM_addStyle((<><![CDATA[
p {
	white-space: pre-wrap;
}
]]></>).toString());

But... pending upstream resolution, I see there is already one kludge in our CSS:
/**
 * Add word wrapping for commit messages, so horizontal scrolling isn't needed.
 * --2012-06-09
 */
.GJEA35ODLB {
	white-space: pre-wrap !important;
}

So my agenda on the matter is:
(1) to add right now a .GJEA35ODOC css element, so we can fix it on our install ;
(2) submit a patch doing the same upstream (because I've the feeling we aren't the only project to put code in comments there...).
Comment 4 Dereckson 2012-08-29 16:44:46 UTC
Gerrit change #21895
Comment 5 Dereckson 2012-09-07 07:21:53 UTC
Merged locally.
Comment 6 Dereckson 2013-09-17 12:43:49 UTC
I'm reopening it, we need to reapply this change against .commentPanelMessage.
Comment 7 Gerrit Notification Bot 2013-09-17 12:49:26 UTC
Change 84522 had a related patch set uploaded by Dereckson:
Break lines in review Gerrit comments

https://gerrit.wikimedia.org/r/84522
Comment 8 Gerrit Notification Bot 2013-10-11 18:44:54 UTC
Change 84522 merged by ArielGlenn:
Break lines in review Gerrit comments

https://gerrit.wikimedia.org/r/84522

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links