Re: [JGIT PATCH 2/3] Cleanup of Branch command ready for verbose mode

[Marek Zawirski <marek.zawirski@gmail.com>]

Charles O'Farrell wrote:
> diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java
(...)
> @@ -87,17 +91,27 @@ private void list() {
>  		if (head != null) {
>  			String current = head.getName();
>  			if (current.equals(Constants.HEAD))
> -				printHead("(no branch)", true);
> -			for (String ref : new TreeSet<String>(refs.keySet())) {
> -				if (isHead(ref))
> -					printHead(ref, current.equals(ref));
> +				addRef("(no branch)", head);
> +			addRefs(refs, Constants.HEADS_PREFIX, !remote);
> +			addRefs(refs, Constants.REMOTES_PREFIX, remote);

We used to use (Constants.HEADS_PREFIX + '/') or 
(Constants.REMOTES_PREFIX + '/') in such places, probably to handle 
correctly jokes like refs named "refs/remotes_will_broke_your_code".

I've seen this expression so many times that I think it's right moment 
to create another Constants.HEADS_PREFIX_SLASHED (same for tags, 
remotes) or similar as this piece of this code is redundant in many 
places. But wait, does anybody use pure ones without slashes? Maybe we 
can just change existing constants.

Beside of that detail, me (being just jgit developer) says that the 
series looks good. Oh, and it's probably good to follow convention of 
marking variables as final when they are final.

-- 
Marek Zawirski [zawir]
marek.zawirski@gmail.com