Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

Re: [EGIT PATCH] showing commiter and parent commit(s) on the revision detail viewer

From: Robin Rosenberg <hidden>
Date: 2016-06-15 22:44:08

Hi Roger,

The general idea is ok. Some minor coments though.
Re: [EGIT PATCH] showing commiter and parent commit(s) on the revision detail viewer
"Show" and final period.

fredagen den 25 januari 2008 skrev Roger C. Soares:
quoted hunk ↗ jump to hunk
@@ -221,14 +223,16 @@ public class GitHistoryPage extends HistoryPage implements IAdaptable,
 				if(selection2.length == 1) {
 					// if the table item is not visible in the UI and it's selected via keyboard
 					// this listener is called before the listener that sets the item data.
-					if(selection2[0] == null) {
+					GitCommitFileRevision revision = (GitCommitFileRevision) selection2[0];
+					if(revision == null) {
 						int ix = table.getSelectionIndex();
-						GitFileRevision revision = (GitFileRevision) fileRevisions.get(ix);
+						revision = (GitCommitFileRevision) fileRevisions.get(ix);
 						selection2[0] = revision;
 					}
-					setRevisionInfoTextViewers(selection2[0]);
+					setRevisionInfoTextViewers(revision);
I'm rather allergic to casts, so if we could have only one I'd feel better. Something like

					IFileRevision revision = selection2[0];
					if(revision == null) {
 						int ix = table.getSelectionIndex();
						revision = fileRevisions.get(ix);
 						selection2[0] = revision;
 					}
					setRevisionInfoTextViewers((GitCommitFileRevision)revision);
 				}
 
+
two lines of whitepspace
 		if (revisionInfo.length() == 0) {
-			revisionInfo.append("Commit: ");
+			revisionInfo.append("Commit ID: ");
I prefer just commit. It's shorter and that it is the id is obvious.
+		revisionInfo.append(formatPersonIdentForRevInfo(commit.getAuthor()));
+		revisionInfo.append("\nCommiter: ");
Two t's in "Committer"
+				} catch (IOException e) {
+					e.printStackTrace();
We should start doing error handling better. But that's a separate chapter. Add a // TODO for now. 
I'll address them one all one one go later, unless someone does it for me.

-- robin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help