My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 816: File indentation is wrong in diff
14 people starred this issue and may be notified of changes. Back to list
Status:  Submitted
Owner:  gustaf.l...@sonyericsson.com
Closed:  Oct 2012


Sign in to add a comment
 
Reported by jessewil...@google.com, Jan 4, 2011
I saw an indentation error in a file and commented on it in gerrit. But the code's author didn't make that error! It was a rendering bug somewhere in gerrit.

I've attached a screenshot of what I saw - note that the 'managedConn.close()' line is not indented from the containing try/catch block.

If you download that file from Gerrit (via the Download link on that page) you see that the line is in fact indented.

                            // BEGIN android-changed
                            try {
                                managedConn.close();
                            } catch (IOException ignored) {
                                // SSLSocket's will throw IOException
                                // because they can't send a "close
                                // notify" protocol message to the
                                // server. Just supresss any
                                // exceptions related to closing the
                                // stale connection.
                            }
                            // END android-changed

So gerrit is somehow eating the whitespace, which leads me to make bogus comments on code reviews. And potentially, also to not make useful comments on code reviews.

Note that I couldn't reproduce this in the original author's gerrit+Chrome.
where-did-whitespace-go.png
115 KB   View   Download
Jan 30, 2011
#1 jessewil...@google.com
I've seen it again!
more-imagined-whitespace-in-gerrit.png
175 KB   View   Download
Feb 2, 2011
#2 jessewil...@google.com
The indentation is wrong in my Chrome (9.0.597.84 beta) but not Brian's. It's also wrong in Firefox 3.16.13.

Another example is in this file:
http://.../g/#patch,sidebyside,94678,3,luni/src/main/native/NativeCrypto.cpp
Sep 22, 2011
#3 sanforda...@gmail.com
I currently have this issue with an Objective-C patch in Firefox 7.0, but not in Chrome 15.0.874.21 dev.

Our self-hosted Gerrit is at 2.2.1.
Feb 14, 2012
#4 dar...@gmail.com
We run into this problem on Gerrit 2.2.1. What we found out is that if you set "Ignore Whitespace:" in the top left corner to "None", the indentation is wrong.

With anything else but "None" (eg. All, At Line End), it displays correctly.

BTW I think this is not a minor bug, it should be escalated a bit, it caused a lot of confusion and problem within our team.

Please verify that this is the same behaviour in your case too. This could help devs to pinpoint the error.
Jun 15, 2012
#5 ern...@gmail.com
I've also seen this.
With Ignore Whitespace: None it's completely wrong.
With anything else the formatting is correct, but the indented lines are still not marked as changed with green color.
Jun 18, 2012
#6 JBjo...@gmail.com
Still happens on 2.4

Oct 5, 2012
Project Member #7 gustaf.l...@sonyericsson.com
(No comment was entered for this change.)
Status: ChangeUnderReview
Owner: gustaf.l...@sonyericsson.com
Oct 12, 2012
#9 sop@google.com
(No comment was entered for this change.)
Labels: FixedIn-2.6
Oct 12, 2012
#10 sop@google.com
(No comment was entered for this change.)
Status: Submitted
Sign in to add a comment

Powered by Google Project Hosting