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

From: Marek Zawirski <marek.zawirski@gmail.com>
Date: Aug 18, 2008, 13:41 UTC
Message-ID: <48A97C23.8030103@gmail.com>
In-Reply-To: <1219057305-9284-3-git-send-email-charleso@charleso.org>

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

(...)

Show 11 quoted lines

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