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

[Robin Rosenberg <robin.rosenberg.lists@dewire.com>]

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:
> @@ -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