I just marked a patch CNW for comments exceeding 80 chars but they were actually 79. Seems like the markers could use 2-3 chars of padding?

Comments

sun’s picture

Hm. This might be a browser/platform-specific rendering/font-size issue.

If you happen to run across this issue once more, please post a link to the affected patch/comment, so we can double-check and debug.

tstoeckler’s picture

I don't know if this is the same issue, but it seems related:
In the patch at http://drupal.org/node/1414314#comment-5512228 in the first hunk of path.admin.inc there is a comment, which according to Dreditor wraps early. It doesn't really, though.

I'm on FF9.0.1/Win7.

sun’s picture

StatusFileSize
new19.23 KB

This is what I see for the mentioned hunk in that patch:

dreditor-80chars.png

sun’s picture

Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs manual testing

The actual code + 80 chars delimiter in ASCII:

                                                                                |
  // Enable language column if language.module is enabled or if we have any     |
  // alias with a language.                                                     |
                                                                                |
  // Enable language column if language.module is enabled or if we have any alia|
                                                                                |

Does not wrap too early, and at least for me, identical view/result in patch reviewer.

tstoeckler’s picture

StatusFileSize
new24.43 KB
new16.51 KB
new17.72 KB

Well, this is in fact a font-size issue. Apparently I'm viewing the browser larger than the rest of the world.
I attached screenshots of the same hunk in my

- my normal viewing size (which seems to be default +1)
dreditor-80-chars.png

- a smaller viewing size (~default)
dreditor-80-chars-smaller.png

- a larger viewing size (~default +2)
dreditor-80-chars-larger.png

sun’s picture

Title: Counts patch +/- lines towards 80 char limit » 80 chars delimiter is not always at 80 chars (due to font-size)
Status: Postponed (maintainer needs more info) » Active

That makes sense. Need to use relative units everywhere. (ugh, that's going to get a 5th point decimal digit thing...)

scor’s picture

StatusFileSize
new136.64 KB

I just RTBC'ed a patch which looked good on my dreditor but which in fact had docs going over the 80 chars limit: http://drupal.org/node/1089040#comment-5653258

This is how it looks, even after updating to the latest dreditor from the project page:

1287794_7_80_char_dreditor.png

Update: and changing the font size on FF (Command + +/-) does not change anything.

sun’s picture

Apparently, the CSS uses em already. :(

We need a CSS expert who's smarter than me to figure out why the location of the delimiter changes on browser zooming, despite using a relative unit already.

markhalliwell’s picture