233695 - [PatchReader] Lines beginning with "+" or "-" disappear in new diff function

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Mike Pinkerton (not reading bugmail)]

Look at the http://bugzilla.mozilla.org/attachment.cgi?id=140802&action=view in
the diff viewer:

http://bugzilla.mozilla.org/attachment.cgi?id=140802&action=diff

If you scroll down to just after the routine "isPantherOrLater" you will see a
function body without a function header. This is because the function header
begins with a "+" symbol (in obj-c, this denotes a static method). You can see
what it should be by looking at the patch itself, which is correct.

Comment 1

[Max Kanat-Alexander]

This is interesting... John, do you think that this is a bug in PatchReader or
in our Bugzilla code?

Comment 2

[Frédéric Buclin]

*** Bug 338486 has been marked as a duplicate of this bug. ***

Comment 4

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070306 SeaMonkey/1.5a] (nightly) (W2Ksp4)

Comment 0 stands, with current b.m.o BugZilla version, whatever it is.

The missing line from the patch is:
{{
++ (nsMacCursor *) createCursor: (enum nsCursor) aCursor
}}

Comment 5

[Chris Lawson (gone)]

Attachment 190249 [details] [diff] (in bug 288038) fixes this problem.

John, is there any plan to land that patch in Patch Reader?

cl

Comment 6

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/20070515 SeaMonkey/1.5a] (nightly) (W2Ksp4)

Bug still there, after "2008-03-27 bmo upgrade" (tracked by bug 425607).
(That's BugZilla v3.0.3 !?)

Comment 7

[Frédéric Buclin]

Not a blocker. This is a bug in PatchReader itself, not in Bugzilla. And I don't want to implement a workaround in a hurry, especially on a stable branch.

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> Attachment 190249 [details] [diff] (in bug 288038) fixes this problem.

Chris,
can you confirm that that patch does fix this bug (too),
and not (only) bug 303683 ?

See bug 288038 comment 9.

> John, is there any plan to land that patch in Patch Reader?

John, ping ?

Comment 10

[Serge Gautherie (:sgautherie)]

FWIW, I filed
n°35362 : PatchReader-0.9.5 bug report: ++/-- line issues
<http://rt.cpan.org/Public/Bug/Display.html?id=35362>

Comment 11

[Chris Lawson (gone)]

(In reply to comment #9)
> (In reply to comment #5)
> > Attachment 190249 [details] [diff] [details] (in bug 288038) fixes this problem.
> 
> Chris,
> can you confirm that that patch does fix this bug (too),
> and not (only) bug 303683 ?
> 
> See bug 288038 comment 9.

I can't recall if I actually tested it or if I just read it and understood it to be true. Someone with a Bugzilla test install should probably verify that it does indeed fix things as it's supposed to.

cl

Comment 12

[[e-mail]]

This is particularly annoying now that the default view for patches is diff mode; I thought attachment 458039 [details] [diff] [review] was corrupt, but in fact it's fine.

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

bmo has taken local patches to PatchViewer before to fix pain points (e.g. bug 362767); if attachment 190249 [details] [diff] [review] in bug 288038 really does fix this problem, can we take it on bmo, too?

This can affect not just Obj-C code, but also jar manifests in certain cases (e.g. attachment 474602 [details] [diff] [review]).  Particularly as Diff view is now the default view in Bugzilla for most references to patches, this bug is becoming more and more annoying.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This also affects HTML close-comment markers, such as in attachment 493143 [details] [diff] [review].

Comment 15

[Frédéric Buclin]

Fixed in PatchReader 0.9.6.

Comment 16

[Max Kanat-Alexander]

Great. We should require that version, no?

Comment 17

[Frédéric Buclin]

(In reply to comment #16)
> Great. We should require that version, no?

Sure, I already planned to file a bug about this, for 4.2. :)