{"thread":{"id":"15067","subject":"[JGIT PATCH 0/3] jgit: Verbase branch command","startedAt":"2008-08-18T11:01:42Z","lastAt":"2008-08-18T14:20:39Z","messageCount":6,"participants":["Charles O'Farrell","Marek Zawirski","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"87527","messageId":"1219057305-9284-1-git-send-email-charleso@charleso.org","threadId":"15067","inReplyTo":null,"subject":"[JGIT PATCH 0/3] jgit: Verbase branch command","fromName":"Charles O'Farrell","fromEmail":"charleso@charleso.org","sentAt":"2008-08-18T11:01:42Z","receivedAt":"2008-08-18T11:01:42Z","isPatch":true,"sender":{"key":"charleso@charleso.org","avatar":"https://gravatar.com/avatar/ce96d531daa37646a38a81e48113865af86a6d290ad92931f3b217896bdf86ea?d=mp&s=160"},"body":"Verbose branch listing added for the sake of completeness\n\nCharles O'Farrell (3):\n  Extract RefComparator to sort collection of Refs\n  Cleanup of Branch command ready for verbose mode\n  Verbose branch command\n\n .../src/org/spearce/jgit/pgm/Branch.java           |   60 +++++++++++++----\n .../src/org/spearce/jgit/lib/RefComparator.java    |   72 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/RefWriter.java        |   14 +----\n 3 files changed, 120 insertions(+), 26 deletions(-)\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefComparator.java\n"},{"id":"87529","messageId":"1219057305-9284-2-git-send-email-charleso@charleso.org","threadId":"15067","inReplyTo":"1219057305-9284-1-git-send-email-charleso@charleso.org","subject":"[JGIT PATCH 1/3] Extract RefComparator to sort collection of Refs","fromName":"Charles O'Farrell","fromEmail":"charleso@charleso.org","sentAt":"2008-08-18T11:01:43Z","receivedAt":"2008-08-18T11:01:43Z","isPatch":true,"sender":{"key":"charleso@charleso.org","avatar":"https://gravatar.com/avatar/ce96d531daa37646a38a81e48113865af86a6d290ad92931f3b217896bdf86ea?d=mp&s=160"},"body":"Signed-off-by: Charles O'Farrell <charleso@charleso.org>\n---\n .../src/org/spearce/jgit/lib/RefComparator.java    |   72 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/RefWriter.java        |   14 +----\n 2 files changed, 73 insertions(+), 13 deletions(-)\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefComparator.java\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefComparator.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefComparator.java\nnew file mode 100644\nindex 0000000..6d994eb\n--- /dev/null\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefComparator.java\n@@ -0,0 +1,72 @@\n+/**\n+ * Copyright (C) 2008, Charles O'Farrell <charleso@charleso.org>\n+ *\n+ * All rights reserved.\n+ *\n+ * Redistribution and use in source and binary forms, with or\n+ * without modification, are permitted provided that the following\n+ * conditions are met:\n+ *\n+ * - Redistributions of source code must retain the above copyright\n+ *   notice, this list of conditions and the following disclaimer.\n+ *\n+ * - Redistributions in binary form must reproduce the above\n+ *   copyright notice, this list of conditions and the following\n+ *   disclaimer in the documentation and/or other materials provided\n+ *   with the distribution.\n+ *\n+ * - Neither the name of the Git Development Community nor the\n+ *   names of its contributors may be used to endorse or promote\n+ *   products derived from this software without specific prior\n+ *   written permission.\n+ *\n+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND\n+ * CONTRIBUTORS \"AS IS\" AND ANY EXPRESS OR IMPLIED WARRANTIES,\n+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES\n+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE\n+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR\n+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\n+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT\n+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;\n+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER\n+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,\n+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)\n+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF\n+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n+ */\n+\n+package org.spearce.jgit.lib;\n+\n+import java.util.ArrayList;\n+import java.util.Collection;\n+import java.util.Collections;\n+import java.util.Comparator;\n+import java.util.List;\n+\n+/**\n+ * Util for sorting (or comparing) Ref instances by name.\n+ * <p>\n+ * Useful for command line tools or writing out refs to file.\n+ */\n+public class RefComparator implements Comparator<Ref> {\n+\n+\t/** Singleton instance of RefComparator */\n+\tpublic static final RefComparator INSTANCE = new RefComparator();\n+\n+\tpublic int compare(Ref o1, Ref o2) {\n+\t\treturn o1.getName().compareTo(o2.getName());\n+\t}\n+\n+\t/**\n+\t * Sorts the collection of refs, returning a new collection.\n+\t * \n+\t * @param refs\n+\t *            collection to be sorted\n+\t * @return sorted collection of refs\n+\t */\n+\tpublic static Collection<Ref> sort(final Collection<Ref> refs) {\n+\t\tfinal List<Ref> r = new ArrayList<Ref>(refs);\n+\t\tCollections.sort(r, INSTANCE);\n+\t\treturn r;\n+\t}\n+}\n\\ No newline at end of file\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java\nindex 9c784d5..2d39713 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java\n@@ -41,8 +41,6 @@\n import java.io.IOException;\n import java.io.StringWriter;\n import java.util.Collection;\n-import java.util.Comparator;\n-import java.util.TreeSet;\n \n /**\n  * Writes out refs to the {@link Constants#INFO_REFS} and\n@@ -61,8 +59,7 @@\n \t *            by applying updates to the advertised refs already discovered.\n \t */\n \tpublic RefWriter(Collection<Ref> refs) {\n-\t\tthis.refs = new TreeSet<Ref>(RefComparator.INSTANCE);\n-\t\tthis.refs.addAll(refs);\n+\t\tthis.refs = RefComparator.sort(refs);\n \t}\n \n \t/**\n@@ -163,13 +160,4 @@ public void writePackedRefs() throws IOException {\n \t */\n \tprotected abstract void writeFile(String file, byte[] content)\n \t\t\tthrows IOException;\n-\n-\tprivate static class RefComparator implements Comparator<Ref> {\n-\n-\t\tprivate static final RefComparator INSTANCE = new RefComparator();\n-\n-\t\tpublic int compare(Ref o1, Ref o2) {\n-\t\t\treturn o1.getName().compareTo(o2.getName());\n-\t\t}\n-\t}\n }\n-- \n1.6.0.2.g2ebc0\n"},{"id":"87528","messageId":"1219057305-9284-3-git-send-email-charleso@charleso.org","threadId":"15067","inReplyTo":"1219057305-9284-2-git-send-email-charleso@charleso.org","subject":"[JGIT PATCH 2/3] Cleanup of Branch command ready for verbose mode","fromName":"Charles O'Farrell","fromEmail":"charleso@charleso.org","sentAt":"2008-08-18T11:01:44Z","receivedAt":"2008-08-18T11:01:44Z","isPatch":true,"sender":{"key":"charleso@charleso.org","avatar":"https://gravatar.com/avatar/ce96d531daa37646a38a81e48113865af86a6d290ad92931f3b217896bdf86ea?d=mp&s=160"},"body":"Signed-off-by: Charles O'Farrell <charleso@charleso.org>\n---\n .../src/org/spearce/jgit/pgm/Branch.java           |   30 ++++++++++++++-----\n 1 files changed, 22 insertions(+), 8 deletions(-)\n\ndiff --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\nindex c89f510..9141be0 100644\n--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java\n+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java\n@@ -39,15 +39,17 @@\n \n import java.io.IOException;\n import java.util.ArrayList;\n+import java.util.LinkedHashMap;\n import java.util.List;\n import java.util.Map;\n-import java.util.TreeSet;\n+import java.util.Map.Entry;\n \n import org.kohsuke.args4j.Argument;\n import org.kohsuke.args4j.Option;\n import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.ObjectId;\n import org.spearce.jgit.lib.Ref;\n+import org.spearce.jgit.lib.RefComparator;\n import org.spearce.jgit.lib.RefUpdate;\n import org.spearce.jgit.lib.RefUpdate.Result;\n \n@@ -72,6 +74,8 @@\n \t@Argument\n \tprivate List<String> branches = new ArrayList<String>();\n \n+\tprivate Map<String, Ref> printRefs = new LinkedHashMap<String, Ref>();\n+\n \t@Override\n \tprotected void run() throws Exception {\n \t\tif (delete || deleteForce)\n@@ -87,17 +91,27 @@ private void list() {\n \t\tif (head != null) {\n \t\t\tString current = head.getName();\n \t\t\tif (current.equals(Constants.HEAD))\n-\t\t\t\tprintHead(\"(no branch)\", true);\n-\t\t\tfor (String ref : new TreeSet<String>(refs.keySet())) {\n-\t\t\t\tif (isHead(ref))\n-\t\t\t\t\tprintHead(ref, current.equals(ref));\n+\t\t\t\taddRef(\"(no branch)\", head);\n+\t\t\taddRefs(refs, Constants.HEADS_PREFIX, !remote);\n+\t\t\taddRefs(refs, Constants.REMOTES_PREFIX, remote);\n+\t\t\tfor (Entry<String, Ref> e : printRefs.entrySet()) {\n+\t\t\t\tprintHead(e.getKey(), current.equals(e.getValue().getName()));\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tprivate void addRefs(Map<String, Ref> allRefs, String prefix, boolean add) {\n+\t\tif (all || add) {\n+\t\t\tfor (Ref ref : RefComparator.sort(allRefs.values())) {\n+\t\t\t\tString name = ref.getName();\n+\t\t\t\tif (name.startsWith(prefix))\n+\t\t\t\t\taddRef(name, ref);\n \t\t\t}\n \t\t}\n \t}\n \n-\tprivate boolean isHead(String key) {\n-\t\treturn (all || !remote) && key.startsWith(Constants.HEADS_PREFIX)\n-\t\t\t\t|| (all || remote) && key.startsWith(Constants.REMOTES_PREFIX);\n+\tprivate void addRef(String name, Ref ref) {\n+\t\tprintRefs.put(name, ref);\n \t}\n \n \tprivate void printHead(String ref, boolean isCurrent) {\n-- \n1.6.0.2.g2ebc0\n"},{"id":"87530","messageId":"1219057305-9284-4-git-send-email-charleso@charleso.org","threadId":"15067","inReplyTo":"1219057305-9284-3-git-send-email-charleso@charleso.org","subject":"[JGIT PATCH 3/3] Verbose branch command","fromName":"Charles O'Farrell","fromEmail":"charleso@charleso.org","sentAt":"2008-08-18T11:01:45Z","receivedAt":"2008-08-18T11:01:45Z","isPatch":true,"sender":{"key":"charleso@charleso.org","avatar":"https://gravatar.com/avatar/ce96d531daa37646a38a81e48113865af86a6d290ad92931f3b217896bdf86ea?d=mp&s=160"},"body":"Signed-off-by: Charles O'Farrell <charleso@charleso.org>\n---\n .../src/org/spearce/jgit/pgm/Branch.java           |   34 +++++++++++++++----\n 1 files changed, 27 insertions(+), 7 deletions(-)\n\ndiff --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\nindex 9141be0..ba8ffde 100644\n--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java\n+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java\n@@ -52,6 +52,7 @@\n import org.spearce.jgit.lib.RefComparator;\n import org.spearce.jgit.lib.RefUpdate;\n import org.spearce.jgit.lib.RefUpdate.Result;\n+import org.spearce.jgit.revwalk.RevWalk;\n \n @Command(common = true, usage = \"List, create, or delete branches\")\n class Branch extends TextBuiltin {\n@@ -76,15 +77,23 @@\n \n \tprivate Map<String, Ref> printRefs = new LinkedHashMap<String, Ref>();\n \n+\t/** Only set for verbose branch listing at-the-moment */\n+\tprivate RevWalk rw;\n+\n+\tprivate int maxNameLength;\n+\n \t@Override\n \tprotected void run() throws Exception {\n \t\tif (delete || deleteForce)\n \t\t\tdelete(deleteForce);\n-\t\telse\n+\t\telse {\n+\t\t\tif (verbose)\n+\t\t\t\trw = new RevWalk(db);\n \t\t\tlist();\n+\t\t}\n \t}\n \n-\tprivate void list() {\n+\tprivate void list() throws Exception {\n \t\tMap<String, Ref> refs = db.getAllRefs();\n \t\tRef head = refs.get(Constants.HEAD);\n \t\t// This can happen if HEAD is stillborn\n@@ -95,7 +104,8 @@ private void list() {\n \t\t\taddRefs(refs, Constants.HEADS_PREFIX, !remote);\n \t\t\taddRefs(refs, Constants.REMOTES_PREFIX, remote);\n \t\t\tfor (Entry<String, Ref> e : printRefs.entrySet()) {\n-\t\t\t\tprintHead(e.getKey(), current.equals(e.getValue().getName()));\n+\t\t\t\tRef ref = e.getValue();\n+\t\t\t\tprintHead(e.getKey(), current.equals(ref.getName()), ref);\n \t\t\t}\n \t\t}\n \t}\n@@ -105,20 +115,30 @@ private void addRefs(Map<String, Ref> allRefs, String prefix, boolean add) {\n \t\t\tfor (Ref ref : RefComparator.sort(allRefs.values())) {\n \t\t\t\tString name = ref.getName();\n \t\t\t\tif (name.startsWith(prefix))\n-\t\t\t\t\taddRef(name, ref);\n+\t\t\t\t\taddRef(name.substring(name.indexOf('/', 5) + 1), ref);\n \t\t\t}\n \t\t}\n \t}\n \n \tprivate void addRef(String name, Ref ref) {\n \t\tprintRefs.put(name, ref);\n+\t\tmaxNameLength = Math.max(maxNameLength, name.length());\n \t}\n \n-\tprivate void printHead(String ref, boolean isCurrent) {\n+\tprivate void printHead(String ref, boolean isCurrent, Ref refObj)\n+\t\t\tthrows Exception {\n \t\tout.print(isCurrent ? '*' : ' ');\n \t\tout.print(' ');\n-\t\tref = ref.substring(ref.indexOf('/', 5) + 1);\n-\t\tout.println(ref);\n+\t\tout.print(ref);\n+\t\tif (verbose) {\n+\t\t\tint spaces = maxNameLength - ref.length() + 1;\n+\t\t\tout.print(String.format(\"%\" + spaces + \"s\", \"\"));\n+\t\t\tObjectId objectId = refObj.getObjectId();\n+\t\t\tout.print(objectId.toString().substring(0, 7));\n+\t\t\tout.print(' ');\n+\t\t\tout.print(rw.parseCommit(objectId).getShortMessage());\n+\t\t}\n+\t\tout.println();\n \t}\n \n \tprivate void delete(boolean force) throws IOException {\n-- \n1.6.0.2.g2ebc0\n"},{"id":"87539","messageId":"48A97C23.8030103@gmail.com","threadId":"15067","inReplyTo":"1219057305-9284-3-git-send-email-charleso@charleso.org","subject":"Re: [JGIT PATCH 2/3] Cleanup of Branch command ready for verbose mode","fromName":"Marek Zawirski","fromEmail":"marek.zawirski@gmail.com","sentAt":"2008-08-18T13:41:55Z","receivedAt":"2008-08-18T13:41:55Z","isPatch":true,"sender":{"key":"marek.zawirski@gmail.com","avatar":null},"body":"Charles O'Farrell wrote:\n> 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\n(...)\n> @@ -87,17 +91,27 @@ private void list() {\n>  \t\tif (head != null) {\n>  \t\t\tString current = head.getName();\n>  \t\t\tif (current.equals(Constants.HEAD))\n> -\t\t\t\tprintHead(\"(no branch)\", true);\n> -\t\t\tfor (String ref : new TreeSet<String>(refs.keySet())) {\n> -\t\t\t\tif (isHead(ref))\n> -\t\t\t\t\tprintHead(ref, current.equals(ref));\n> +\t\t\t\taddRef(\"(no branch)\", head);\n> +\t\t\taddRefs(refs, Constants.HEADS_PREFIX, !remote);\n> +\t\t\taddRefs(refs, Constants.REMOTES_PREFIX, remote);\n\nWe used to use (Constants.HEADS_PREFIX + '/') or \n(Constants.REMOTES_PREFIX + '/') in such places, probably to handle \ncorrectly jokes like refs named \"refs/remotes_will_broke_your_code\".\n\nI've seen this expression so many times that I think it's right moment \nto create another Constants.HEADS_PREFIX_SLASHED (same for tags, \nremotes) or similar as this piece of this code is redundant in many \nplaces. But wait, does anybody use pure ones without slashes? Maybe we \ncan just change existing constants.\n\nBeside of that detail, me (being just jgit developer) says that the \nseries looks good. Oh, and it's probably good to follow convention of \nmarking variables as final when they are final.\n\n-- \nMarek Zawirski [zawir]\nmarek.zawirski@gmail.com\n"},{"id":"87541","messageId":"20080818142039.GA9572@spearce.org","threadId":"15067","inReplyTo":"48A97C23.8030103@gmail.com","subject":"Re: [JGIT PATCH 2/3] Cleanup of Branch command ready for verbose mode","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-18T14:20:39Z","receivedAt":"2008-08-18T14:20:39Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Marek Zawirski <marek.zawirski@gmail.com> wrote:\n> Charles O'Farrell wrote:\n>> 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\n> (...)\n>> @@ -87,17 +91,27 @@ private void list() {\n>>  \t\tif (head != null) {\n>>  \t\t\tString current = head.getName();\n>>  \t\t\tif (current.equals(Constants.HEAD))\n>> -\t\t\t\tprintHead(\"(no branch)\", true);\n>> -\t\t\tfor (String ref : new TreeSet<String>(refs.keySet())) {\n>> -\t\t\t\tif (isHead(ref))\n>> -\t\t\t\t\tprintHead(ref, current.equals(ref));\n>> +\t\t\t\taddRef(\"(no branch)\", head);\n>> +\t\t\taddRefs(refs, Constants.HEADS_PREFIX, !remote);\n>> +\t\t\taddRefs(refs, Constants.REMOTES_PREFIX, remote);\n>\n> We used to use (Constants.HEADS_PREFIX + '/') or  \n> (Constants.REMOTES_PREFIX + '/') in such places, probably to handle  \n> correctly jokes like refs named \"refs/remotes_will_broke_your_code\".\n\nYup.  I hate those constants because they are almost useless.\nAlmost anytime we need them, we really need to use instead\n(Constants.FOO_PREFIX + '/').\n\n> I've seen this expression so many times that I think it's right moment  \n> to create another Constants.HEADS_PREFIX_SLASHED (same for tags,  \n> remotes) or similar as this piece of this code is redundant in many  \n> places. But wait, does anybody use pure ones without slashes? Maybe we  \n> can just change existing constants.\n\nRight.  It would be a nice cleanup to go through the existing users\nand see if we cannot change the meaning of these.  But that's a\nlot of code to change because you also have to delete the +'/'\nwe have everywhere.\n\n> Beside of that detail, me (being just jgit developer) says that the  \n> series looks good. Oh, and it's probably good to follow convention of  \n> marking variables as final when they are final.\n\nYea, the series does look nice, except the prefix matching bug.\n\n-- \nShawn.\n"}]}