* [JGIT PATCH 6/6] Add ~user friendly Bourne style quoting for TransportGitSsh
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1228946751-12708-6-git-send-email-spearce@spearce.org>
This mostly completes the migration of our quoting rules from the
SSH transport to our QuotedString pattern. User names may be left
alone for the shell to expand when the string is evaluated, if the
caller wants that sort of behavior in a particular context.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../util/QuotedStringBourneUserPathStyleTest.java | 130 ++++++++++++++++++++
.../spearce/jgit/transport/TransportGitSsh.java | 27 +----
.../src/org/spearce/jgit/util/QuotedString.java | 28 ++++
3 files changed, 160 insertions(+), 25 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneUserPathStyleTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneUserPathStyleTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneUserPathStyleTest.java
new file mode 100644
index 0000000..36fb52a
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneUserPathStyleTest.java
@@ -0,0 +1,130 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import static org.spearce.jgit.util.QuotedString.BOURNE_USER_PATH;
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+
+public class QuotedStringBourneUserPathStyleTest extends TestCase {
+ private static void assertQuote(final String in, final String exp) {
+ final String r = BOURNE_USER_PATH.quote(in);
+ assertNotSame(in, r);
+ assertFalse(in.equals(r));
+ assertEquals('\'' + exp + '\'', r);
+ }
+
+ private static void assertDequote(final String exp, final String in) {
+ final byte[] b = Constants.encode('\'' + in + '\'');
+ final String r = BOURNE_USER_PATH.dequote(b, 0, b.length);
+ assertEquals(exp, r);
+ }
+
+ public void testQuote_Empty() {
+ assertEquals("''", BOURNE_USER_PATH.quote(""));
+ }
+
+ public void testDequote_Empty1() {
+ assertEquals("", BOURNE_USER_PATH.dequote(new byte[0], 0, 0));
+ }
+
+ public void testDequote_Empty2() {
+ assertEquals("", BOURNE_USER_PATH.dequote(new byte[] { '\'', '\'' }, 0,
+ 2));
+ }
+
+ public void testDequote_SoleSq() {
+ assertEquals("", BOURNE_USER_PATH.dequote(new byte[] { '\'' }, 0, 1));
+ }
+
+ public void testQuote_BareA() {
+ assertQuote("a", "a");
+ }
+
+ public void testDequote_BareA() {
+ final String in = "a";
+ final byte[] b = Constants.encode(in);
+ assertEquals(in, BOURNE_USER_PATH.dequote(b, 0, b.length));
+ }
+
+ public void testDequote_BareABCZ_OnlyBC() {
+ final String in = "abcz";
+ final byte[] b = Constants.encode(in);
+ final int p = in.indexOf('b');
+ assertEquals("bc", BOURNE_USER_PATH.dequote(b, p, p + 2));
+ }
+
+ public void testDequote_LoneBackslash() {
+ assertDequote("\\", "\\");
+ }
+
+ public void testQuote_NamedEscapes() {
+ assertQuote("'", "'\\''");
+ assertQuote("!", "'\\!'");
+
+ assertQuote("a'b", "a'\\''b");
+ assertQuote("a!b", "a'\\!'b");
+ }
+
+ public void testDequote_NamedEscapes() {
+ assertDequote("'", "'\\''");
+ assertDequote("!", "'\\!'");
+
+ assertDequote("a'b", "a'\\''b");
+ assertDequote("a!b", "a'\\!'b");
+ }
+
+ public void testQuote_User() {
+ assertEquals("~foo/", BOURNE_USER_PATH.quote("~foo"));
+ assertEquals("~foo/", BOURNE_USER_PATH.quote("~foo/"));
+ assertEquals("~/", BOURNE_USER_PATH.quote("~/"));
+
+ assertEquals("~foo/'a'", BOURNE_USER_PATH.quote("~foo/a"));
+ assertEquals("~/'a'", BOURNE_USER_PATH.quote("~/a"));
+ }
+
+ public void testDequote_User() {
+ assertEquals("~foo", BOURNE_USER_PATH.dequote("~foo"));
+ assertEquals("~foo/", BOURNE_USER_PATH.dequote("~foo/"));
+ assertEquals("~/", BOURNE_USER_PATH.dequote("~/"));
+
+ assertEquals("~foo/a", BOURNE_USER_PATH.dequote("~foo/'a'"));
+ assertEquals("~/a", BOURNE_USER_PATH.dequote("~/'a'"));
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index e3f5ae8..d4bf466 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -131,31 +131,8 @@ private static void sqAlways(final StringBuilder cmd, final String val) {
}
private static void sq(final StringBuilder cmd, final String val) {
- int i = 0;
-
- if (val.length() == 0)
- return;
- if (val.matches("^~[A-Za-z0-9_-]+$")) {
- // If the string is just "~user" we can assume they
- // mean "~user/" and evaluate it within the shell.
- //
- cmd.append(val);
- cmd.append('/');
- return;
- }
-
- if (val.matches("^~[A-Za-z0-9_-]*/.*$")) {
- // If the string is of "~/path" or "~user/path"
- // we must not escape ~/ or ~user/ from the shell
- // as we need that portion to be evaluated.
- //
- i = val.indexOf('/') + 1;
- cmd.append(val.substring(0, i));
- if (i == val.length())
- return;
- }
-
- cmd.append(QuotedString.BOURNE.quote(val.substring(i)));
+ if (val.length() > 0)
+ cmd.append(QuotedString.BOURNE.quote(val));
}
private void initSession() throws TransportException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java b/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
index 1089e9e..d2f71b4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
@@ -57,6 +57,9 @@
*/
public static final BourneStyle BOURNE = new BourneStyle();
+ /** Bourne style, but permits <code>~user</code> at the start of the string. */
+ public static final BourneUserPathStyle BOURNE_USER_PATH = new BourneUserPathStyle();
+
/**
* Quote an input string by the quoting rules.
* <p>
@@ -176,6 +179,31 @@ public String dequote(final byte[] in, int ip, final int ie) {
}
}
+ /** Bourne style, but permits <code>~user</code> at the start of the string. */
+ public static class BourneUserPathStyle extends BourneStyle {
+ @Override
+ public String quote(final String in) {
+ if (in.matches("^~[A-Za-z0-9_-]+$")) {
+ // If the string is just "~user" we can assume they
+ // mean "~user/".
+ //
+ return in + "/";
+ }
+
+ if (in.matches("^~[A-Za-z0-9_-]*/.*$")) {
+ // If the string is of "~/path" or "~user/path"
+ // we must not escape ~/ or ~user/ from the shell.
+ //
+ final int i = in.indexOf('/') + 1;
+ if (i == in.length())
+ return in;
+ return in.substring(0, i) + super.quote(in.substring(i));
+ }
+
+ return super.quote(in);
+ }
+ }
+
/** Quoting style that obeys the rules of the C programming language. */
public static final class C_Style extends QuotedString {
private static final byte[] quote;
--
1.6.1.rc2.299.gead4c
^ permalink raw reply related
* [JGIT PATCH 5/6] Add Bourne style quoting for TransportGitSsh
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1228946751-12708-5-git-send-email-spearce@spearce.org>
Now that we have a nice QuotedString abstraction we can port our
string quoting logic from being private within the SSH transport
code to being available in the rest of the library.
Currently we only support the super-restrictive quoting style used
for the repository path name argument over SSH. We don't support the
"minimal" style used to invoke the command name, nor do we support
the ~user/ style format, which cannot be quoted.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../jgit/util/QuotedStringBourneStyleTest.java | 111 ++++++++++++++++++++
.../spearce/jgit/transport/TransportGitSsh.java | 13 +--
.../src/org/spearce/jgit/util/QuotedString.java | 66 ++++++++++++
3 files changed, 179 insertions(+), 11 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneStyleTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneStyleTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneStyleTest.java
new file mode 100644
index 0000000..86d46fe
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneStyleTest.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import static org.spearce.jgit.util.QuotedString.BOURNE;
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+
+public class QuotedStringBourneStyleTest extends TestCase {
+ private static void assertQuote(final String in, final String exp) {
+ final String r = BOURNE.quote(in);
+ assertNotSame(in, r);
+ assertFalse(in.equals(r));
+ assertEquals('\'' + exp + '\'', r);
+ }
+
+ private static void assertDequote(final String exp, final String in) {
+ final byte[] b = Constants.encode('\'' + in + '\'');
+ final String r = BOURNE.dequote(b, 0, b.length);
+ assertEquals(exp, r);
+ }
+
+ public void testQuote_Empty() {
+ assertEquals("''", BOURNE.quote(""));
+ }
+
+ public void testDequote_Empty1() {
+ assertEquals("", BOURNE.dequote(new byte[0], 0, 0));
+ }
+
+ public void testDequote_Empty2() {
+ assertEquals("", BOURNE.dequote(new byte[] { '\'', '\'' }, 0, 2));
+ }
+
+ public void testDequote_SoleSq() {
+ assertEquals("", BOURNE.dequote(new byte[] { '\'' }, 0, 1));
+ }
+
+ public void testQuote_BareA() {
+ assertQuote("a", "a");
+ }
+
+ public void testDequote_BareA() {
+ final String in = "a";
+ final byte[] b = Constants.encode(in);
+ assertEquals(in, BOURNE.dequote(b, 0, b.length));
+ }
+
+ public void testDequote_BareABCZ_OnlyBC() {
+ final String in = "abcz";
+ final byte[] b = Constants.encode(in);
+ final int p = in.indexOf('b');
+ assertEquals("bc", BOURNE.dequote(b, p, p + 2));
+ }
+
+ public void testDequote_LoneBackslash() {
+ assertDequote("\\", "\\");
+ }
+
+ public void testQuote_NamedEscapes() {
+ assertQuote("'", "'\\''");
+ assertQuote("!", "'\\!'");
+
+ assertQuote("a'b", "a'\\''b");
+ assertQuote("a!b", "a'\\!'b");
+ }
+
+ public void testDequote_NamedEscapes() {
+ assertDequote("'", "'\\''");
+ assertDequote("!", "'\\!'");
+
+ assertDequote("a'b", "a'\\''b");
+ assertDequote("a!b", "a'\\!'b");
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index 3f2cd37..e3f5ae8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -47,6 +47,7 @@
import org.spearce.jgit.errors.NoRemoteRepositoryException;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.util.QuotedString;
import com.jcraft.jsch.ChannelExec;
import com.jcraft.jsch.JSchException;
@@ -154,17 +155,7 @@ private static void sq(final StringBuilder cmd, final String val) {
return;
}
- cmd.append('\'');
- for (; i < val.length(); i++) {
- final char c = val.charAt(i);
- if (c == '\'')
- cmd.append("'\\''");
- else if (c == '!')
- cmd.append("'\\!'");
- else
- cmd.append(c);
- }
- cmd.append('\'');
+ cmd.append(QuotedString.BOURNE.quote(val.substring(i)));
}
private void initSession() throws TransportException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java b/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
index 4aaa8ff..1089e9e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
@@ -49,6 +49,15 @@
public static final C_Style C = new C_Style();
/**
+ * Quoting style used by the Bourne shell.
+ * <p>
+ * Quotes are unconditionally inserted during {@link #quote(String)}. This
+ * protects shell meta-characters like <code>$</code> or <code>~</code> from
+ * being recognized as special.
+ */
+ public static final BourneStyle BOURNE = new BourneStyle();
+
+ /**
* Quote an input string by the quoting rules.
* <p>
* If the input string does not require any quoting, the same String
@@ -110,6 +119,63 @@ public String dequote(final String in) {
*/
public abstract String dequote(byte[] in, int offset, int end);
+ /**
+ * Quoting style used by the Bourne shell.
+ * <p>
+ * Quotes are unconditionally inserted during {@link #quote(String)}. This
+ * protects shell meta-characters like <code>$</code> or <code>~</code> from
+ * being recognized as special.
+ */
+ public static class BourneStyle extends QuotedString {
+ @Override
+ public String quote(final String in) {
+ final StringBuilder r = new StringBuilder();
+ r.append('\'');
+ int start = 0, i = 0;
+ for (; i < in.length(); i++) {
+ switch (in.charAt(i)) {
+ case '\'':
+ case '!':
+ r.append(in, start, i);
+ r.append('\'');
+ r.append('\\');
+ r.append(in.charAt(i));
+ r.append('\'');
+ start = i + 1;
+ break;
+ }
+ }
+ r.append(in, start, i);
+ r.append('\'');
+ return r.toString();
+ }
+
+ @Override
+ public String dequote(final byte[] in, int ip, final int ie) {
+ boolean inquote = false;
+ final byte[] r = new byte[ie - ip];
+ int rPtr = 0;
+ while (ip < ie) {
+ final byte b = in[ip++];
+ switch (b) {
+ case '\'':
+ inquote = !inquote;
+ continue;
+ case '\\':
+ if (inquote || ip == ie)
+ r[rPtr++] = b; // literal within a quote
+ else
+ r[rPtr++] = in[ip++];
+ continue;
+ default:
+ r[rPtr++] = b;
+ continue;
+ }
+ }
+ return decode(Constants.CHARSET, r, 0, rPtr);
+ }
+ }
+
/** Quoting style that obeys the rules of the C programming language. */
public static final class C_Style extends QuotedString {
private static final byte[] quote;
--
1.6.1.rc2.299.gead4c
^ permalink raw reply related
* [JGIT PATCH 4/6] Add QuotedString class to handle C-style quoting rules
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1228946751-12708-4-git-send-email-spearce@spearce.org>
Git patch files can contain file names which are quoted using the
C language quoting rules. In order to correctly create or parse
these files we must implement a quoting style that matches those
specific rules.
QuotedString itself is an abstract API so callers can be passed a
quoting style based on the context of where their output will be
used, and multiple styles could be supported. This may be useful
if jgit ever grows a "git for-each-ref" style of output where Perl,
Python, Tcl and Bourne style quoting might be necessary.
References through the singleton QuotedString.C should be able to
bypass the virtual function table, as the specific type is mentioned
in the field declaration and that type is final. A good JIT should
be able to remove the abstraction costs when the caller has hardcoded
the quoting style.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/jgit/util/QuotedStringC_StyleTest.java | 144 +++++++++++
.../src/org/spearce/jgit/util/QuotedString.java | 270 ++++++++++++++++++++
2 files changed, 414 insertions(+), 0 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringC_StyleTest.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringC_StyleTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringC_StyleTest.java
new file mode 100644
index 0000000..493869c
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringC_StyleTest.java
@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import static org.spearce.jgit.util.QuotedString.C;
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+
+public class QuotedStringC_StyleTest extends TestCase {
+ private static void assertQuote(final String in, final String exp) {
+ final String r = C.quote(in);
+ assertNotSame(in, r);
+ assertFalse(in.equals(r));
+ assertEquals('"' + exp + '"', r);
+ }
+
+ private static void assertDequote(final String exp, final String in) {
+ final byte[] b = Constants.encode('"' + in + '"');
+ final String r = C.dequote(b, 0, b.length);
+ assertEquals(exp, r);
+ }
+
+ public void testQuote_Empty() {
+ assertEquals("\"\"", C.quote(""));
+ }
+
+ public void testDequote_Empty1() {
+ assertEquals("", C.dequote(new byte[0], 0, 0));
+ }
+
+ public void testDequote_Empty2() {
+ assertEquals("", C.dequote(new byte[] { '"', '"' }, 0, 2));
+ }
+
+ public void testDequote_SoleDq() {
+ assertEquals("\"", C.dequote(new byte[] { '"' }, 0, 1));
+ }
+
+ public void testQuote_BareA() {
+ final String in = "a";
+ assertSame(in, C.quote(in));
+ }
+
+ public void testDequote_BareA() {
+ final String in = "a";
+ final byte[] b = Constants.encode(in);
+ assertEquals(in, C.dequote(b, 0, b.length));
+ }
+
+ public void testDequote_BareABCZ_OnlyBC() {
+ final String in = "abcz";
+ final byte[] b = Constants.encode(in);
+ final int p = in.indexOf('b');
+ assertEquals("bc", C.dequote(b, p, p + 2));
+ }
+
+ public void testDequote_LoneBackslash() {
+ assertDequote("\\", "\\");
+ }
+
+ public void testQuote_NamedEscapes() {
+ assertQuote("\u0007", "\\a");
+ assertQuote("\b", "\\b");
+ assertQuote("\f", "\\f");
+ assertQuote("\n", "\\n");
+ assertQuote("\r", "\\r");
+ assertQuote("\t", "\\t");
+ assertQuote("\u000B", "\\v");
+ assertQuote("\\", "\\\\");
+ assertQuote("\"", "\\\"");
+ }
+
+ public void testDequote_NamedEscapes() {
+ assertDequote("\u0007", "\\a");
+ assertDequote("\b", "\\b");
+ assertDequote("\f", "\\f");
+ assertDequote("\n", "\\n");
+ assertDequote("\r", "\\r");
+ assertDequote("\t", "\\t");
+ assertDequote("\u000B", "\\v");
+ assertDequote("\\", "\\\\");
+ assertDequote("\"", "\\\"");
+ }
+
+ public void testDequote_OctalAll() {
+ for (int i = 0; i < 256; i++) {
+ String s = Integer.toOctalString(i);
+ while (s.length() < 3) {
+ s = "0" + s;
+ }
+ assertDequote("" + (char) i, "\\" + s);
+ }
+ }
+
+ public void testQuote_OctalAll() {
+ assertQuote("\1", "\\001");
+ assertQuote("~", "\\176");
+ assertQuote("\u00ff", "\\303\\277"); // \u00ff in UTF-8
+ }
+
+ public void testDequote_UnknownEscapeQ() {
+ assertDequote("\\q", "\\q");
+ }
+
+ public void testDequote_FooTabBar() {
+ assertDequote("foo\tbar", "foo\\tbar");
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java b/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
new file mode 100644
index 0000000..4aaa8ff
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
@@ -0,0 +1,270 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import static org.spearce.jgit.util.RawParseUtils.decode;
+
+import java.util.Arrays;
+
+import org.spearce.jgit.lib.Constants;
+
+/** Utility functions related to quoted string handling. */
+public abstract class QuotedString {
+ /** Quoting style that obeys the rules of the C programming language. */
+ public static final C_Style C = new C_Style();
+
+ /**
+ * Quote an input string by the quoting rules.
+ * <p>
+ * If the input string does not require any quoting, the same String
+ * reference is returned to the caller.
+ * <p>
+ * Otherwise a quoted string is returned, including the opening and closing
+ * quotation marks at the start and end of the string. If the style does not
+ * permit raw Unicode characters then the string will first be encoded in
+ * UTF-8, with unprintable sequences possibly escaped by the rules.
+ *
+ * @param in
+ * any non-null Unicode string.
+ * @return a quoted string. See above for details.
+ */
+ public abstract String quote(String in);
+
+ /**
+ * Clean a previously quoted input, decoding the result via UTF-8.
+ * <p>
+ * This method must match quote such that:
+ *
+ * <pre>
+ * a.equals(dequote(quote(a)));
+ * </pre>
+ *
+ * is true for any <code>a</code>.
+ *
+ * @param in
+ * a Unicode string to remove quoting from.
+ * @return the cleaned string.
+ * @see #dequote(byte[], int, int)
+ */
+ public String dequote(final String in) {
+ final byte[] b = Constants.encode(in);
+ return dequote(b, 0, b.length);
+ }
+
+ /**
+ * Decode a previously quoted input, scanning a UTF-8 encoded buffer.
+ * <p>
+ * This method must match quote such that:
+ *
+ * <pre>
+ * a.equals(dequote(Constants.encode(quote(a))));
+ * </pre>
+ *
+ * is true for any <code>a</code>.
+ * <p>
+ * This method removes any opening/closing quotation marks added by
+ * {@link #quote(String)}.
+ *
+ * @param in
+ * the input buffer to parse.
+ * @param offset
+ * first position within <code>in</code> to scan.
+ * @param end
+ * one position past in <code>in</code> to scan.
+ * @return the cleaned string.
+ */
+ public abstract String dequote(byte[] in, int offset, int end);
+
+ /** Quoting style that obeys the rules of the C programming language. */
+ public static final class C_Style extends QuotedString {
+ private static final byte[] quote;
+ static {
+ quote = new byte[128];
+ Arrays.fill(quote, (byte) -1);
+
+ for (int i = '0'; i <= '9'; i++)
+ quote[i] = 0;
+ for (int i = 'a'; i <= 'z'; i++)
+ quote[i] = 0;
+ for (int i = 'A'; i <= 'Z'; i++)
+ quote[i] = 0;
+ quote[' '] = 0;
+ quote['+'] = 0;
+ quote[','] = 0;
+ quote['-'] = 0;
+ quote['.'] = 0;
+ quote['/'] = 0;
+ quote['='] = 0;
+ quote['_'] = 0;
+ quote['^'] = 0;
+
+ quote['\u0007'] = 'a';
+ quote['\b'] = 'b';
+ quote['\f'] = 'f';
+ quote['\n'] = 'n';
+ quote['\r'] = 'r';
+ quote['\t'] = 't';
+ quote['\u000B'] = 'v';
+ quote['\\'] = '\\';
+ quote['"'] = '"';
+ }
+
+ @Override
+ public String quote(final String instr) {
+ if (instr.length() == 0)
+ return "\"\"";
+ boolean reuse = true;
+ final byte[] in = Constants.encode(instr);
+ final StringBuilder r = new StringBuilder(2 + in.length);
+ r.append('"');
+ for (int i = 0; i < in.length; i++) {
+ final int c = in[i] & 0xff;
+ if (c < quote.length) {
+ final byte style = quote[c];
+ if (style == 0) {
+ r.append((char) c);
+ continue;
+ }
+ if (style > 0) {
+ reuse = false;
+ r.append('\\');
+ r.append((char) style);
+ continue;
+ }
+ }
+
+ reuse = false;
+ r.append('\\');
+ r.append((char) (((c >> 6) & 03) + '0'));
+ r.append((char) (((c >> 3) & 07) + '0'));
+ r.append((char) (((c >> 0) & 07) + '0'));
+ }
+ if (reuse)
+ return instr;
+ r.append('"');
+ return r.toString();
+ }
+
+ @Override
+ public String dequote(final byte[] in, final int inPtr, final int inEnd) {
+ if (2 <= inEnd - inPtr && in[inPtr] == '"' && in[inEnd - 1] == '"')
+ return dq(in, inPtr + 1, inEnd - 1);
+ return decode(Constants.CHARSET, in, inPtr, inEnd);
+ }
+
+ private static String dq(final byte[] in, int inPtr, final int inEnd) {
+ final byte[] r = new byte[inEnd - inPtr];
+ int rPtr = 0;
+ while (inPtr < inEnd) {
+ final byte b = in[inPtr++];
+ if (b != '\\') {
+ r[rPtr++] = b;
+ continue;
+ }
+
+ if (inPtr == inEnd) {
+ // Lone trailing backslash. Treat it as a literal.
+ //
+ r[rPtr++] = '\\';
+ break;
+ }
+
+ switch (in[inPtr++]) {
+ case 'a':
+ r[rPtr++] = 0x07 /* \a = BEL */;
+ continue;
+ case 'b':
+ r[rPtr++] = '\b';
+ continue;
+ case 'f':
+ r[rPtr++] = '\f';
+ continue;
+ case 'n':
+ r[rPtr++] = '\n';
+ continue;
+ case 'r':
+ r[rPtr++] = '\r';
+ continue;
+ case 't':
+ r[rPtr++] = '\t';
+ continue;
+ case 'v':
+ r[rPtr++] = 0x0B/* \v = VT */;
+ continue;
+
+ case '\\':
+ case '"':
+ r[rPtr++] = in[inPtr - 1];
+ continue;
+
+ case '0':
+ case '1':
+ case '2':
+ case '3': {
+ int cp = in[inPtr - 1] - '0';
+ while (inPtr < inEnd) {
+ final byte c = in[inPtr];
+ if ('0' <= c && c <= '7') {
+ cp <<= 3;
+ cp |= c - '0';
+ inPtr++;
+ } else {
+ break;
+ }
+ }
+ r[rPtr++] = (byte) cp;
+ continue;
+ }
+
+ default:
+ // Any other code is taken literally.
+ //
+ r[rPtr++] = '\\';
+ r[rPtr++] = in[inPtr - 1];
+ continue;
+ }
+ }
+
+ return decode(Constants.CHARSET, r, 0, rPtr);
+ }
+
+ private C_Style() {
+ // Singleton
+ }
+ }
+}
--
1.6.1.rc2.299.gead4c
^ permalink raw reply related
* [JGIT PATCH 2/6] Simplify RawParseUtils next and nextLF loops
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1228946751-12708-2-git-send-email-spearce@spearce.org>
We always need ptr + 1 after we read the current position,
so we might as well do the much more common foo[ptr++]. A
good JIT would be more likely to optimize this case over
the weird else branch we had.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/util/RawParseUtils.java | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index 10c2239..5a40911 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -221,10 +221,8 @@ public static final int parseTimeZoneOffset(final byte[] b, int ptr) {
public static final int next(final byte[] b, int ptr, final char chrA) {
final int sz = b.length;
while (ptr < sz) {
- if (b[ptr] == chrA)
- return ptr + 1;
- else
- ptr++;
+ if (b[ptr++] == chrA)
+ return ptr;
}
return ptr;
}
@@ -260,11 +258,9 @@ public static final int nextLF(final byte[] b, int ptr) {
public static final int nextLF(final byte[] b, int ptr, final char chrA) {
final int sz = b.length;
while (ptr < sz) {
- final byte c = b[ptr];
+ final byte c = b[ptr++];
if (c == chrA || c == '\n')
- return ptr + 1;
- else
- ptr++;
+ return ptr;
}
return ptr;
}
--
1.6.1.rc2.299.gead4c
^ permalink raw reply related
* [JGIT PATCH 3/6] Correct Javadoc of RawParseUtils next and nextLF methods
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1228946751-12708-3-git-send-email-spearce@spearce.org>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/util/RawParseUtils.java | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index 5a40911..8896d38 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -213,10 +213,10 @@ public static final int parseTimeZoneOffset(final byte[] b, int ptr) {
* @param b
* buffer to scan.
* @param ptr
- * position within buffer to start looking for LF at.
+ * position within buffer to start looking for chrA at.
* @param chrA
* character to find.
- * @return new position just after chr.
+ * @return new position just after chrA.
*/
public static final int next(final byte[] b, int ptr, final char chrA) {
final int sz = b.length;
@@ -250,10 +250,10 @@ public static final int nextLF(final byte[] b, int ptr) {
* @param b
* buffer to scan.
* @param ptr
- * position within buffer to start looking for LF at.
+ * position within buffer to start looking for chrA or LF at.
* @param chrA
* character to find.
- * @return new position just after the first chrA or chrB to be found.
+ * @return new position just after the first chrA or LF to be found.
*/
public static final int nextLF(final byte[] b, int ptr, final char chrA) {
final int sz = b.length;
--
1.6.1.rc2.299.gead4c
^ permalink raw reply related
* [JGIT PATCH 1/6] Simplify RawParseUtils.nextLF invocations
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1228946751-12708-1-git-send-email-spearce@spearce.org>
Most of the time when we call next('\n') or nextLF('\n') we really
meant to just say nextLF(), which is logically identical to next()
but could be micro-optimized for the LF byte.
This refactoring shifts the calls to use the new nextLF wrapper for
next('\n'), so we can later chose to make this optimization, or to
leave the code as-is. But either way the call sites are now much
clearer to read.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/ObjectChecker.java | 4 +-
.../src/org/spearce/jgit/revwalk/RevTag.java | 2 +-
.../src/org/spearce/jgit/util/RawParseUtils.java | 25 ++++++++++++++++----
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
index b303d6f..75e3c77 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
@@ -205,11 +205,11 @@ public void checkTag(final byte[] raw) throws CorruptObjectException {
if ((ptr = match(raw, ptr, type)) < 0)
throw new CorruptObjectException("no type header");
- ptr = nextLF(raw, ptr, '\n');
+ ptr = nextLF(raw, ptr);
if ((ptr = match(raw, ptr, tag)) < 0)
throw new CorruptObjectException("no tag header");
- ptr = nextLF(raw, ptr, '\n');
+ ptr = nextLF(raw, ptr);
if ((ptr = match(raw, ptr, tagger)) < 0)
throw new CorruptObjectException("no tagger header");
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
index bbb18ee..77a55cd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
@@ -90,7 +90,7 @@ void parseCanonical(final RevWalk walk, final byte[] rawTag)
object = walk.lookupAny(walk.idBuffer, oType);
int p = pos.value += 4; // "tag "
- final int nameEnd = RawParseUtils.next(rawTag, p, '\n') - 1;
+ final int nameEnd = RawParseUtils.nextLF(rawTag, p) - 1;
name = RawParseUtils.decode(Constants.CHARSET, rawTag, p, nameEnd);
buffer = rawTag;
flags |= PARSED;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index 4b96439..10c2239 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -230,6 +230,21 @@ public static final int next(final byte[] b, int ptr, final char chrA) {
}
/**
+ * Locate the first position after the next LF.
+ * <p>
+ * This method stops on the first '\n' it finds.
+ *
+ * @param b
+ * buffer to scan.
+ * @param ptr
+ * position within buffer to start looking for LF at.
+ * @return new position just after the first LF found.
+ */
+ public static final int nextLF(final byte[] b, int ptr) {
+ return next(b, ptr, '\n');
+ }
+
+ /**
* Locate the first position after either the given character or LF.
* <p>
* This method stops on the first match it finds from either chrA or '\n'.
@@ -296,7 +311,7 @@ public static final int committer(final byte[] b, int ptr) {
while (ptr < sz && b[ptr] == 'p')
ptr += 48; // skip this parent.
if (ptr < sz && b[ptr] == 'a')
- ptr = next(b, ptr, '\n');
+ ptr = nextLF(b, ptr);
return match(b, ptr, committer);
}
@@ -320,7 +335,7 @@ public static final int encoding(final byte[] b, int ptr) {
return -1;
if (b[ptr] == 'e')
break;
- ptr = next(b, ptr, '\n');
+ ptr = nextLF(b, ptr);
}
return match(b, ptr, encoding);
}
@@ -342,7 +357,7 @@ public static Charset parseEncoding(final byte[] b) {
final int enc = encoding(b, 0);
if (enc < 0)
return Constants.CHARSET;
- final int lf = next(b, enc, '\n');
+ final int lf = nextLF(b, enc);
return Charset.forName(decode(Constants.CHARSET, b, enc, lf - 1));
}
@@ -505,7 +520,7 @@ public static final int commitMessage(final byte[] b, int ptr) {
// header line type is.
//
while (ptr < sz && b[ptr] != '\n')
- ptr = next(b, ptr, '\n');
+ ptr = nextLF(b, ptr);
if (ptr < sz && b[ptr] == '\n')
return ptr + 1;
return -1;
@@ -529,7 +544,7 @@ public static final int endOfParagraph(final byte[] b, final int start) {
int ptr = start;
final int sz = b.length;
while (ptr < sz && b[ptr] != '\n')
- ptr = next(b, ptr, '\n');
+ ptr = nextLF(b, ptr);
while (0 < ptr && start < ptr && b[ptr - 1] == '\n')
ptr--;
return ptr;
--
1.6.1.rc2.299.gead4c
^ permalink raw reply related
* [JGIT PATCH 0/6] RawParseUtil improvements
From: Shawn O. Pearce @ 2008-12-10 22:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
I'm working on patch parsing support for JGit, so I can rely on
it in Gerrit 2. Currently Gerrit 1 uses a bastard chunk of code
I ripped out in an hour to scan the output of "git diff"; its not
suitable for long-term appliction.
This series improves RawParseUtils code clarity, and then adds
support for the C-style quoting rules used by git diff when file
names contain "special" characters like LF.
I'm working on patch parsing right now; but I wanted to send this
preliminary series out so you aren't drowning in code to review.
Shawn O. Pearce (6):
Simplify RawParseUtils.nextLF invocations
Simplify RawParseUtils next and nextLF loops
Correct Javadoc of RawParseUtils next and nextLF methods
Add QuotedString class to handle C-style quoting rules
Add Bourne style quoting for TransportGitSsh
Add ~user friendly Bourne style quoting for TransportGitSsh
.../jgit/util/QuotedStringBourneStyleTest.java | 111 ++++++
.../util/QuotedStringBourneUserPathStyleTest.java | 130 +++++++
.../spearce/jgit/util/QuotedStringC_StyleTest.java | 144 ++++++++
.../src/org/spearce/jgit/lib/ObjectChecker.java | 4 +-
.../src/org/spearce/jgit/revwalk/RevTag.java | 2 +-
.../spearce/jgit/transport/TransportGitSsh.java | 38 +--
.../src/org/spearce/jgit/util/QuotedString.java | 364 ++++++++++++++++++++
.../src/org/spearce/jgit/util/RawParseUtils.java | 45 ++-
8 files changed, 783 insertions(+), 55 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneStyleTest.java
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringBourneUserPathStyleTest.java
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/QuotedStringC_StyleTest.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/QuotedString.java
^ permalink raw reply
* Removing a merge commit
From: Ray Morgan @ 2008-12-10 20:20 UTC (permalink / raw)
To: git
Hello,
To build a release for our site, we merge branches that developers
create. We do this with --no-ff in order to make it only one commit to
pull if it fails QA. Say the qa branch's history has 4 merge commits
in a row, is there any way to remove the 3rd (just pulling it out..
much like how a rebase works)?
Currently we just checkout below that failed branch and re-merge
everything above it.. but that just seems very clumsy (and manual).
Thanks!
Ray
^ permalink raw reply
* Re: [PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()
From: Luben Tuikov @ 2008-12-10 21:15 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200812102203.30486.jnareb@gmail.com>
--- On Wed, 12/10/08, Jakub Narebski <jnareb@gmail.com> wrote:
> You don't need $old_full_rev. We have to save data
> about commit in
> %metainfo hash because information about commit appears in
Oh, yeah, the hash... Then, it looks correct.
Acked-by: Luben Tuikov <ltuikov@yahoo.com>
^ permalink raw reply
* Re: [PATCH] modify/delete conflict resolution overwrites untracked file
From: Clemens Buchacher @ 2008-12-10 21:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzlj3ycr4.fsf@gitster.siamese.dyndns.org>
On Wed, Dec 10, 2008 at 12:51:59PM -0800, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
>
> > If it's a regression, it dates far back, since 1.5.0 fails as well.
>
> A good lit(h)mus test to see if it is a regression or just a plain bug in
> the recursive strategy would be to see what 'resolve' strategy does
> (replace "merge" with "merge -s resolve" in your test).
"merge -s resolve" fails with
Trying really trivial in-index merge...
error: Merge requires file-level merging
Nope.
Trying simple merge.
Simple merge failed, trying Automatic merge.
ERROR: c1.c: Not handling case ae9304576a6ec3419b231b2b9c8e33a06f97f9fb ->
-> 8173b675dc61bb578b411c769c9fb654625a7c4e
fatal: merge program failed
Automatic merge failed; fix conflicts and then commit the result.
and therefore passes the test.
^ permalink raw reply
* Re: [PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()
From: Jakub Narebski @ 2008-12-10 21:03 UTC (permalink / raw)
To: Luben Tuikov; +Cc: git
In-Reply-To: <710873.22344.qm@web31806.mail.mud.yahoo.com>
On Wed, 10 Dec 2008, Luben Tuikov wrote:
> --- On Wed, 12/10/08, Jakub Narebski <jnareb@gmail.com> wrote:
Side note: is it Yahoo web mail interface that removes attributions?
> > > Have you tested this patch that it gives the same commit chain
> > > as before it?
> >
> > The only difference between precious version and this patch
> > is that now, if you calculate sha-1 of $long_rev^, it is stored in
> > $metainfo{$long_rev}{'parent'} and not calculated second time.
>
> Yes, I agree a patch to this effect would improve performance
> proportionally to the history of the lines of a file.
Or rather proportionally to the ratio of number of lines of a file
to number of unique commits (not groups of lines) which are blamed
for given contents of a file.
> So it's a good thing, as most commits change a contiguous block
> of size more than one line of a file.
>
> "$parent_commit" depends on "$full_rev^" which depends on "$full_rev".
> So as soon as "$full_rev" != "$old_full_rev", you'd know that you need
> to update "$parent_commit". "$old_full_rev" needs to exist outside
> the scope of "while (1)". I didn't see this in the code or in the
> patch.
You don't need $old_full_rev. We have to save data about commit in
%metainfo hash because information about commit appears in
"git blame --porcelain" output _once_ per commit. So we use the same
cache to store $full_rev^ in $meta{'parent'}, which means storing
it in $metainfo{$full_rev}{'parent'}.
Now if the commit we saved this info about appears again in git-blame
output, be it in group of lines for which the same commit is blamed,
or later in unrelated chunk, we use saved info.
Let me try to explain it using the following diagram:
Commit N Line Original code This patch
------------------------------------------------------
3a4046 1 xxx rev-parse 3a4046^ rev-parse 3a4046^
2 xxx rev-parse 3a4046^ $mi{3a4046}{'parent'}
3 xxx rev-parse 3a4046^ $mi{3a4046}{'parent'}
f47c19 5 xxx rev-parse f47c19^ rev-parse f47c19^
6 xxx rev-parse f47c19^ $mi{f47c19}{'parent'}
3a4046 7 xxx rev-parse 3a4046^ $mi{3a4046}{'parent'} <--
8 xxx rev-parse 3a4046^ $mi{3a4046}{'parent'}
where "rev-parse 3a4046^" means call to git-rev-parse, and $mi{<rev>}
accessing $metainfo{$full_rev} (via $meta).
In place marked by arrow '<--' you don't need to calculate 3a4046^
again...
> > But I have checked that (at least for single example file)
> > the blame output is identical for before and after this patch.
>
> I've always tested it on something like "gitweb.perl", etc.
I've checked it on blob.h. Other good example is README (with boundary
commits) and GIT-VERSION-GEN (with different output between git-blame
--porcelain and --incremental), both of which take much less time than
gitweb/gitweb.perl (see benchmarks in other post).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] modify/delete conflict resolution overwrites untracked file
From: Junio C Hamano @ 2008-12-10 20:51 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git
In-Reply-To: <20081210201259.GA12928@localhost>
Clemens Buchacher <drizzd@aon.at> writes:
> If it's a regression, it dates far back, since 1.5.0 fails as well.
A good lit(h)mus test to see if it is a regression or just a plain bug in
the recursive strategy would be to see what 'resolve' strategy does
(replace "merge" with "merge -s resolve" in your test).
^ permalink raw reply
* Re: [StGit] kha/{safe,experimental} updated
From: Karl Hasselström @ 2008-12-10 20:40 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Gustav Hållberg, David Kågedal
In-Reply-To: <20081208203923.GA9986@diana.vm.bytemark.co.uk>
On 2008-12-08 21:39:23 +0100, Karl Hasselström wrote:
> The "safe" branch has a whole bunch of stgit.el improvements by
> David and Gustav. "experimental" has the same two patches that just
> sit there waiting for the git features they depend on to be
> sufficiently widely deployed.
Updated with three more stgit.el patches from Gustav. (One of them
only in experimental, since Gustav seemed to think it was a bit
hackish, and David hasn't commented on it yet, and I'm an elisp noob.)
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()
From: Junio C Hamano @ 2008-12-10 20:27 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Nanako Shiraishi, git, Luben Tuikov
In-Reply-To: <200812101439.18526.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> On Wed, 10 Dec 2008, Nanako Shiraishi wrote:
>> Quoting Jakub Narebski <jnareb@gmail.com>:
>>
>> > Unfortunately the implementation in 244a70e used one call for
>> > git-rev-parse to find parent revision per line in file, instead of
>> > using long lived "git cat-file --batch-check" (which might not existed
>> > then), or changing validate_refname to validate_revision and made it
>> > accept <rev>^, <rev>^^, <rev>^^^ etc. syntax.
>>
>> Could you substantiate why this is "Unfortunate"?
>
> Because it calls git-rev-parse once for _each line_, even if for lines
> in the group of neighbour lines blamed by same commit $parent_commit
> is the same, and even if you need to calculate $parent_commit only once
> per unique individual commit present in blame output.
It probably was obvious that this was meant as a patch for better
performance without changing the functionality. I tend to think the
presentation wasn't so great, though.
Luben Tuikov changed 'lineno' link from leading to commit which lead
to current version of given block of lines, to leading to parent of
this commit in 244a70e (Blame "linenr" link jumps to previous state at
"orig_lineno"). This supposedly made data mining possible (or just
better).
Other than "supposedly" which I do not think should be there, I think this
is a great opening paragraph to establish the context.
Unfortunately the implementation in 244a70e used one call for
git-rev-parse to find parent revision per line in file, instead of
using long lived "git cat-file --batch-check" (which might not existed
then), or changing validate_refname to validate_revision and made it
accept <rev>^, <rev>^^, <rev>^^^ etc. syntax.
But I do not think this is such a great second paragraph that states what
problem it tries to solve. I'd say something like this instead:
The current implementation calls rev-parse once per line to find
parent revision of blamed commit, even when the same commit
appears more than once, which is inefficient.
And then the outline of the solution:
This patch attempts to migitate issue a bit by caching $parent_commit
info in %metainfo, which makes gitweb to call git-rev-parse only once
per unique commit in blame output.
which is very good, except that I do not think you need to say "a bit".
And have your benchmark (two tables and footnotes) after this outline of
the solution.
I think the first part of "Unfortunately" paragraph can be dropped
(because that is already in the first half of problem description) and the
rest can come as the last paragraph as "Possible future enhancements".
> Appendix A:
> ~~~~~~~~~~~
> #!/bin/bash
>
> export GATEWAY_INTERFACE="CGI/1.1"
> export HTTP_ACCEPT="*/*"
> export REQUEST_METHOD="GET"
> export QUERY_STRING=""$1""
> export PATH_INFO=""$2""
>
> export GITWEB_CONFIG="/home/jnareb/git/gitweb/gitweb_config.perl"
>
> perl -- /home/jnareb/git/gitweb/gitweb.perl
>
> # end of gitweb-run.sh
I'd suggest making a separate patch to add "gitweb-run.sh" in contrib/ so
that other people can use it when checking their changes to gitweb. The
script might need some more polishing, though. For example, it is not
very obvious if you have *_config.perl only to customize for your
environment (e.g. where the test repositories are) or you need to have
some overrides in there when you are running gitweb as a standalone script.
To recap, I think the commit log for this patch would have been much
easier to read if it were presented in this order:
a paragraph to establish the context;
a paragraph to state what problem it tries to solve;
a paragraph (or more) to explain the solution; and finally
a paragraph to discuss possible future enhancements.
^ permalink raw reply
* Re: [PATCH 2/2] diff: respect textconv in rewrite diffs
From: Junio C Hamano @ 2008-12-10 20:27 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20081210090221.GA11367@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But more importantly, the fixup you just pushed seems to have an extra
> ">dump":
Oops, sorry.
^ permalink raw reply
* Re: after first git clone of linux kernel repository there are changed files in working dir
From: Brett Simmers @ 2008-12-10 20:20 UTC (permalink / raw)
To: rdkrsr; +Cc: git
In-Reply-To: <d304880b0812101022u2abe5d68ub3bda68ed39f830b@mail.gmail.com>
On Wed, Dec 10, 2008 at 1:22 PM, rdkrsr <rdkrsr@googlemail.com> wrote:
> I just fetched the sources without changing anything, but git diff
> shows, that there are changes that are not yet updated (changed but not
> updated: use git add to ...). Why is it like that?
>
> I use msysgit on windows, maybe that is one reason?
What are the filenames? I've seen git on Windows get confused if a
repository has two files that are the same except for the case of some
of the letters (since both can't exist by default on NTFS).
-Brett
^ permalink raw reply
* Re: [PATCH v2] git-sh-setup: Fix scripts whose PWD is a symlink into a git work-dir
From: Junio C Hamano @ 2008-12-10 20:18 UTC (permalink / raw)
To: Marcel M. Cary; +Cc: git, jnareb, ae, j.sixt
In-Reply-To: <1228921454-22416-1-git-send-email-marcel@oak.homeunix.org>
"Marcel M. Cary" <marcel@oak.homeunix.org> writes:
> * When interpretting a relative upward (../) path in cd_to_toplevel,
> prepend the cwd without symlinks, given by /bin/pwd
> * Add tests for cd_to_toplevel and "git pull" in a symlinked
> directory that failed before this fix, plus constrasting
> scenarios that already worked
These are descriptions of changes (and good ones at that, but
"constrasting?").
It however is a good idea to describe the problem the patch tries to solve
*before* going into details of what you did. "If A is B, operation C
tries to incorrectly access directory D; it should use directory E. This
breakage is because F is confused by G..."
Yes, the "Subject:" already hints about the "If A is B" part, and the
second bullet point uses the word "failed" to hint that there was a
breakage, but that will not be sufficient description to recall the
analysis you did of the problem, when you have read the commit log message
6 months from now what the breakage was about.
In order to justify the change against "Doctor if A is B, it hurts ---
don't do it then" rebuttals, it further may make sense to defend why it is
sometimes useful to be able to satisify the precondition that triggers the
existing problem. That would come before the problem description to
prepare readers with the context of the patch.
> diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
> new file mode 100755
> index 0000000..293dc35
> --- /dev/null
> +++ b/t/t2300-cd-to-toplevel.sh
> @@ -0,0 +1,37 @@
> +#!/bin/sh
> +
> +test_description='cd_to_toplevel'
> +
> +. ./test-lib.sh
> +
> +test_cd_to_toplevel () {
> + test_expect_success "$2" '
> + (
> + cd '"'$1'"' &&
> + . git-sh-setup &&
> + cd_to_toplevel &&
> + [ "$(pwd -P)" = "$TOPLEVEL" ]
> + )
> + '
> +}
> +
> +TOPLEVEL="$(pwd -P)/repo"
Hmm. Does it make sense to assume everybody's pwd can take -P when the
primary change this patch introduces carefully avoids assuming the
availability of -P for "cd"?
> +ln -s repo symrepo
> +test_cd_to_toplevel symrepo 'at symbolic root'
> +
> +ln -s repo/sub/dir subdir-link
> +test_cd_to_toplevel subdir-link 'at symbolic subdir'
> +
> +cd repo
> +ln -s sub/dir internal-link
> +test_cd_to_toplevel internal-link 'at internal symbolic subdir'
To be very honest, although it is good that you made them work, I am still
not getting why the latter two scenarios are worth supporting. The first
one I am Ok with, though.
> diff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh
> new file mode 100755
> index 0000000..f18fec7
> --- /dev/null
> +++ b/t/t5521-pull-symlink.sh
> @@ -0,0 +1,67 @@
> +#!/bin/sh
> +
> +test_description='pulling from symlinked subdir'
> +
> +. ./test-lib.sh
> +
> +D=`pwd`
> +
> +# The scenario we are building:
> +#
> +# trash\ directory/
> +# clone-repo/
> +# subdir/
> +# bar
> +# subdir-link -> clone-repo/subdir/
> +#
> +# The working directory is subdir-link.
> +#
It is great to see the scenario explained like this. It makes it easier
to follow what the tests are trying to do.
> +test_expect_success setup '
> +
> + mkdir subdir &&
> + touch subdir/bar &&
> + git add subdir/bar &&
> + git commit -m empty &&
> + git clone . clone-repo &&
> + # demonstrate that things work without the symlink
> + test_debug "cd clone-repo/subdir/ && git pull; cd ../.." &&
> + ln -s clone-repo/subdir/ subdir-link &&
> + cd subdir-link/ &&
> + test_debug "set +x"
> +'
> +
> +# From subdir-link, pulling should work as it does from
> +# clone-repo/subdir/.
> +#
> +# Instead, the error pull gave was:
> +#
> +# fatal: 'origin': unable to chdir or not a git archive
> +# fatal: The remote end hung up unexpectedly
> +#
> +# because git would find the .git/config for the "trash directory"
> +# repo, not for the clone-repo repo. The "trash directory" repo
> +# had no entry for origin. Git found the wrong .git because
> +# git rev-parse --show-cdup printed a path relative to
> +# clone-repo/subdir/, not subdir-link/. Git rev-parse --show-cdup
> +# used the correct .git, but when the git pull shell script did
> +# "cd `git rev-parse --show-cdup`", it ended up in the wrong
> +# directory. Shell "cd" works a little different from chdir() in C.
> +# Bash's "cd -P" works like chdir() in C.
This is a very good analysis. s/Bash's "cd -P"/"cd -P" in POSIX shells/,
though.
> +#
> +test_expect_success 'pulling from symlinked subdir' '
> +
> + git pull
> +'
I'd prefer to see each test_expect_success be able to fail independently,
which would mean (1) when you chdir around, do so in a subshell, and (2)
each test_expect_success assumes it begins in the same directory.
In the case of these tests, I think it is just the matter of moving the
last two lines from the previous test to the beginning of this test and
enclosing this test in (), right?
> +
> +# Prove that the remote end really is a repo, and other commands
> +# work fine in this context.
> +#
> +test_debug "
> + test_expect_success 'pushing from symlinked subdir' '
> +
> + git push
> + '
> +"
Why should this be hidden inside test_debug?
^ permalink raw reply
* [PATCH] modify/delete conflict resolution overwrites untracked file
From: Clemens Buchacher @ 2008-12-10 20:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
If a file was removed in HEAD, but modified in MERGE_HEAD, recursive merge
will result in a "CONFLICT (delete/modify)". If the (now untracked) file
already exists and was not added to the index, it is overwritten with the
conflict resolution contents.
In similar situations (cf. test 2), the merge would abort with
"error: Untracked working tree 'file' would be overwritten by merge."
The same should happen in this case.
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
If it's a regression, it dates far back, since 1.5.0 fails as well.
I don't have time to look into this until Saturday. I won't complain if
someone beats me to it. ;-)
Clemens
t/t7607-merge-overwrite.sh | 87 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 87 insertions(+), 0 deletions(-)
create mode 100755 t/t7607-merge-overwrite.sh
diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
new file mode 100755
index 0000000..513097c
--- /dev/null
+++ b/t/t7607-merge-overwrite.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Do not overwrite changes.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo c0 > c0.c &&
+ git add c0.c &&
+ git commit -m c0 &&
+ git tag c0 &&
+ echo c1 > c1.c &&
+ git add c1.c &&
+ git commit -m c1 &&
+ git tag c1 &&
+ git reset --hard c0 &&
+ echo c2 > c2.c &&
+ git add c2.c &&
+ git commit -m c2 &&
+ git tag c2 &&
+ git reset --hard c1 &&
+ echo "c1 a" > c1.c &&
+ git add c1.c &&
+ git commit -m "c1 a" &&
+ git tag c1a &&
+ echo "VERY IMPORTANT CHANGES" > important
+'
+
+test_expect_success 'will not overwrite untracked file' '
+ git reset --hard c1 &&
+ cat important > c2.c &&
+ ! git merge c2 &&
+ test_cmp important c2.c
+'
+
+test_expect_success 'will not overwrite new file' '
+ git reset --hard c1 &&
+ cat important > c2.c &&
+ git add c2.c &&
+ ! git merge c2 &&
+ test_cmp important c2.c
+'
+
+test_expect_success 'will not overwrite staged changes' '
+ git reset --hard c1 &&
+ cat important > c2.c &&
+ git add c2.c &&
+ rm c2.c &&
+ ! git merge c2 &&
+ git checkout c2.c &&
+ test_cmp important c2.c
+'
+
+test_expect_failure 'will not overwrite removed file' '
+ git reset --hard c1 &&
+ git rm c1.c &&
+ git commit -m "rm c1.c" &&
+ cat important > c1.c &&
+ ! git merge c1a &&
+ test_cmp important c1.c
+'
+
+test_expect_success 'will not overwrite re-added file' '
+ git reset --hard c1 &&
+ git rm c1.c &&
+ git commit -m "rm c1.c" &&
+ cat important > c1.c &&
+ git add c1.c &&
+ ! git merge c1a &&
+ test_cmp important c1.c
+'
+
+test_expect_success 'will not overwrite removed file with staged changes' '
+ git reset --hard c1 &&
+ git rm c1.c &&
+ git commit -m "rm c1.c" &&
+ cat important > c1.c &&
+ git add c1.c &&
+ rm c1.c &&
+ ! git merge c1a &&
+ git checkout c1.c &&
+ test_cmp important c1.c
+'
+
+test_done
--
1.6.0
^ permalink raw reply related
* [RFC/PATCH 4/3] gitweb: Incremental blame (proof of concept)
From: Jakub Narebski @ 2008-12-10 20:11 UTC (permalink / raw)
To: git
Cc: Luben Tuikov, Nanako Shiraishi, Petr Baudis, Fredrik Kuivinen,
Fredrik Kuivinen, Petr Baudis, Jakub Narebski
In-Reply-To: <20081209223703.28106.29198.stgit@localhost.localdomain>
This is tweaked up version of Petr Baudis <pasky@suse.cz> patch, which
in turn was tweaked up version of Fredrik Kuivinen <frekui@gmail.com>'s
proof of concept patch. It adds 'blame_incremental' view, which
incrementally displays line data in blame view using JavaScript (AJAX).
The original patch by Fredrik Kuivinen has been lightly tested in a
couple of browsers (Firefox, Mozilla, Konqueror, Galeon, Opera and IE6).
The next patch by Petr Baudis has been tested in Firefox and Epiphany.
This patch has been lightly tested in Mozilla 1.17.2 and Konqueror
3.5.3.
This patch does not (contrary to the one by Petr Baudis) enable this
view in gitweb: there are no links leading to 'blame_incremental'
action. You would have to generate URL 'by hand' (e.g. changing 'blame'
or 'blob' in gitweb URL to 'blame_incremental'). Having links in gitweb
lead to this new action (e.g. by rewriting them like in previous patch),
if JavaScript is enabled in browser, is left for later.
Like earlier patch by Per Baudis it avoids code duplication, but it goes
one step further and use git_blame_common for ordinary blame view, for
incremental blame, and for incremental blame data.
How the 'blame_incremental' view works:
* gitweb generates initial info by putting file contents (from
git-cat-file) together with line numbers in blame table
* then gitweb makes web browser JavaScript engine call startBlame()
function from blame.js
* startBlame() opens connection to 'blame_data' view, which in turn
calls "git blame --incremental" for a file, and streams output of
git-blame to JavaScript (blame.js)
* blame.js updates line info in blame view, coloring it, and updating
progress info; note that it has to use 3 colors to ensure that
different neighbour groups have different styles
* when 'blame_data' ends, and blame.js finishes updating line info,
it fixes colors to match (as far as possible) ordinary 'blame' view,
and updates generating time info.
This code uses http://ajaxpatterns.org/HTTP_Streaming pattern.
It deals with streamed 'blame_data' server error by notifying about them
in the progress info area (just in case).
This patch adds GITWEB_BLAMEJS compile configuration option, and
modifies git-instaweb.sh to take blame.js into account, but it does not
update gitweb/README file (as it is only proof of concept patch). The
code for git-instaweb.sh was taken from Pasky's patch.
While at it, this patch uniquifies td.dark and td.dark2 style: they
differ only in that td.dark2 doesn't have style for :hover.
This patch also adds showing time (in seconds) it took to generate
a page in page footer (based on example code by Pasky), even though
it is independent change, to be able to evaluate incremental blame in
gitweb better. In proper patch series it would be independent commit;
and it probably would provide fallback if Time::HiRes is not available
(by e.g. not showing generating time info), even though this is
unlikely.
Cc: Fredrik Kuivinen <frekui@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I'm sorry if you have received duplicate copy of this message...
NOTE: This patch is RFC proof of concept patch!: it should be split
onto many smaller patches for easy review (and bug finding) in version
meant to be applied.
[Please tell me if some of info below should be put in the commit
message instead of here]
Patch by Petr Baudis this one is based on:
http://permalink.gmane.org/gmane.comp.version-control.git/56657
Original patch by Fredrik Kuivinen:
http://article.gmane.org/gmane.comp.version-control.git/41361
Snippet adding 'generated in' to gitweb, by Petr Baudis:
http://article.gmane.org/gmane.comp.version-control.git/83306
Should I post interdiff to Petr Baudis patch, and comments about
difference between them? This post is quite long as it is now...
Differences between 'blame' and 'blame_incremental' output:
* Line number links in 'blame' lead to parent of blamed commit, i.e. to
the view where given line _changed_; this behavior was introduced in
244a70e (Blame "linenr" link jumps to previous state at "orig_lineno")
by Luben Tuikov on Jan 2008 to make data mining possible.
Currently 'blame_incremental' lead to version at blamed commit; this
would be hard to change without opening another stream, or without
having gitweb accept <sha1>^ for 'hb' (hash_base) parameter.
* The title attribute (shown in popup on mouseover) for "sha1" cell
for 'blame_incremental' view looks like the following:
'Kay Sievers, 2005-08-07 19:49:46 +0000 (21:49:46 +0200)'
more like the date format used in 'commit' view, rather than shorter
'Kay Sievers, 2005-08-07 21:49:46 +0200'
This is a bit of accident, as in originla patch(es) there was error
where the time and date shown were for UTC (GMT), and not for shown
together timezone, i.e.
'Kay Sievers, 2005-08-07 19:49:46 +0200'
and I have added local time instead of replacing it. But perhaps it
is 'blame' view that should change format?
* In 'blame_incremental' the cell (<td>) with sha1 has rowspan
attribute set even if it is at its default value rowspan="1".
This should be very easy to fix.
* The 'blame_incremental' view has new feature inspired by output of
"git gui blame <file>", that if group of lines blamed to the same
commit has more than two lines, then below shortened sha-1 of a
commit it is shown shortened author (initials, in uppercase).
If you think it is worth it, this feature should be fairly easy to
port to ordinary non-incremental 'blame' view.
* Sometimes "git blame --porcelain" and "git blame --incremental" can
give different output. Compare for example 'blame' and
'blame_incremental' view for GIT-VERSION-GEN in git.git repository,
and note that in 'blame_incremental' the last two groups are for the
same commit (compare bottom parts of pages). 'blame_incremental'
currently does not consolidate groups; if it did that, it should do
it before fixColors()
* During filling blame info 'blame_incremental' view uses three colors
(three styles: color1, color2, color3) instead of two color zebra
coloring (two styles: light2, dark2) to ensure that the color of
current group is different from both of its neighbours.
This is non-issue: this is fixed at the end (although intermediate
versions of blame.js script didn't fo fixColors() to make it easier
to check if the 3-coloring is correct).
* The 'blame_incremental' view contains unique progress bar and
progress report; perhaps they should vanish after succesfull loading
of all data?
Bugs and suspected bugs in Mozilla 1.17.2 (my main browser); perhaps
those got corrected, as 1.17.2 is ancient (Gecko/20050923) version:
* HTML 4.01 Transitional specification at W3C states only ID and NAME
tokens must begin with a letter ([A-Za-z]); it looks like class
named "1" or "2" or "3" has style specified by CSS ignored.
* With XHTML 1.0 DTD and application/xhtml+xml content type, if there
were <col /> elements in blame table (currently commented out in the
source), then row with column headings (with <td> elements) was not
visible.
* With XHTML 1.0 DTD and application/xhtml+xml content type, if there
was an error in JavaScript, instead of having it visible as error
message in JavaScript Console, Mozilla behaved as if the script
wasn't there at all.
* With XHTML 1.0 DTD and application/xhtml+xml content type, setting
innerHTML doesn't work: it raises cryptic JavaScript exception:
Component returned failure code: 0x80004005 (NS_ERROR_FAILURE)
[nsIDOMNSHTMLElement.innerHTML]
Correct solution would be to use only DOM, or rather depending on
what web browser supports, use either .innerHTML (which is
proprietary Microsoft extension) or DOM (which is standard but not
all browser use it). Current *workaround* is to simply always use
'text/html' content type, and HTML 4.01 DTD.
I wonder whether innerHTML or DOM is faster; and how many of web
browser implements other similar properties like innerText,
outerHTML and insertAdjacentHTML.
* XHTML 1.0 doesn't allow for trick with using HTML comments to hide
contents of <script> element from old browsers, as XML compliant
browser can remove XML comments before seeing JavaScript, so we
don't use it. Besides I think this issue is irrelevant now.
P.S. I have removed a few spurious one line change chunks from
gitweb/gitweb.perl, which were done to not confuse Perl syntax
highlighting (lazy-lock-mode) from cperl-mode in GNU Emacs 21.4.1,
and to not confuse imenu parser in GNU Emacs (which allow to go to
specified subroutine, and to display which-function-info in
modeline), so diffstat is slightly out of sync.
Those were #" or #' after regexp with single unescaped " or ',
and ($) in definition of "sub S_ISGITLINK($)".
P.P.S. What is the stance for copyrigth assesments in the files
for git code, like the ones in gitweb/gitweb.perl and gitweb/blame.js?
P.P.P.S. Should I use Signed-off-by from Pasky and Fredrik if I based
my code on theirs, and if they all signed their patches?
Makefile | 4 +
git-instaweb.sh | 6 +
gitweb/blame.js | 398 ++++++++++++++++++++++++++++++++++++++++++++++++++++
gitweb/gitweb.css | 27 +++-
gitweb/gitweb.perl | 252 ++++++++++++++++++++++-----------
5 files changed, 597 insertions(+), 90 deletions(-)
create mode 100644 gitweb/blame.js
diff --git a/Makefile b/Makefile
index 5158197..05ac246 100644
--- a/Makefile
+++ b/Makefile
@@ -218,6 +218,7 @@ GITWEB_HOMETEXT = indextext.html
GITWEB_CSS = gitweb.css
GITWEB_LOGO = git-logo.png
GITWEB_FAVICON = git-favicon.png
+GITWEB_BLAMEJS = blame.js
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
@@ -1209,6 +1210,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+ -e 's|++GITWEB_BLAMEJS++|$(GITWEB_BLAMEJS)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
$< >$@+ && \
@@ -1224,6 +1226,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
-e '/@@GITWEB_CGI@@/d' \
-e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
-e '/@@GITWEB_CSS@@/d' \
+ -e '/@@GITWEB_BLAMEJS@@/r gitweb/blame.js' \
+ -e '/@@GITWEB_BLAMEJS@@/d' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
$@.sh > $@+ && \
chmod +x $@+ && \
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 0843372..f41354c 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -268,6 +268,12 @@ gitweb_css () {
EOFGITWEB
}
+gitweb_blamejs () {
+ cat > "$1" <<\EOFGITWEB
+@@GITWEB_BLAMEJS@@
+EOFGITWEB
+}
+
gitweb_cgi "$GIT_DIR/gitweb/gitweb.cgi"
gitweb_css "$GIT_DIR/gitweb/gitweb.css"
diff --git a/gitweb/blame.js b/gitweb/blame.js
new file mode 100644
index 0000000..197d615
--- /dev/null
+++ b/gitweb/blame.js
@@ -0,0 +1,398 @@
+// Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+
+var DEBUG = 0;
+function debug(str) {
+ if (DEBUG)
+ alert(str);
+}
+
+function createRequestObject() {
+ var ro;
+ if (window.XMLHttpRequest) {
+ ro = new XMLHttpRequest();
+ } else {
+ ro = new ActiveXObject("Microsoft.XMLHTTP");
+ }
+ if (!ro)
+ debug("Couldn't start XMLHttpRequest object");
+ return ro;
+}
+
+var http;
+var baseUrl;
+
+// 'commits' is an associative map. It maps SHA1s to Commit objects.
+var commits = new Object();
+
+function Commit(sha1) {
+ this.sha1 = sha1;
+}
+
+// convert month or day of the month to string, padding it with
+// '0' (zero) to two characters width if necessary
+function zeroPad(n) {
+ if (n < 10)
+ return '0' + n;
+ else
+ return n.toString();
+}
+
+function spacePad(n,width) {
+ var scale = 1;
+ var str = '';
+
+ while (width > 1) {
+ scale *= 10;
+ if (n < scale)
+ str += ' ';
+ width--;
+ }
+ return str + n.toString();
+}
+
+
+var blamedLines = 0;
+var totalLines = '???';
+var div_progress_bar;
+var div_progress_info;
+
+function countLines() {
+ var table = document.getElementById('blame_table');
+ if (!table)
+ table = document.getElementsByTagName('table').item(0);
+
+ if (table)
+ return table.getElementsByTagName('tr').length - 1; // for header
+ else
+ return '...';
+}
+
+function updateProgressInfo() {
+ if (!div_progress_info)
+ div_progress_info = document.getElementById('progress_info');
+ if (!div_progress_bar)
+ div_progress_bar = document.getElementById('progress_bar');
+ if (!div_progress_info && !div_progress_bar)
+ return;
+
+ var percentage = Math.floor(100.0*blamedLines/totalLines);
+
+ if (div_progress_info) {
+ div_progress_info.innerHTML = blamedLines + ' / ' + totalLines
+ + ' ('+spacePad(percentage,3)+'%)';
+ }
+
+ if (div_progress_bar) {
+ div_progress_bar.setAttribute('style', 'width: '+percentage+'%;');
+ }
+}
+
+var colorRe = new RegExp('color([0-9]*)');
+/* return N if <tr class="colorN">, otherwise return null */
+function getColorNo(tr) {
+ var className = tr.getAttribute('class');
+ if (className) {
+ match = colorRe.exec(className);
+ if (match)
+ return parseInt(match[1]);
+ }
+ return null;
+}
+
+function findColorNo(tr_prev, tr_next) {
+ var color_prev;
+ var color_next;
+
+ if (tr_prev)
+ color_prev = getColorNo(tr_prev);
+ if (tr_next)
+ color_next = getColorNo(tr_next);
+
+ if (!color_prev && !color_next)
+ return 1;
+ if (color_prev == color_next)
+ return ((color_prev % 3) + 1);
+ if (!color_prev)
+ return ((color_next % 3) + 1);
+ if (!color_next)
+ return ((color_prev % 3) + 1);
+ return (3 - ((color_prev + color_next) % 3));
+}
+
+var tzRe = new RegExp('^([+-][0-9][0-9])([0-9][0-9])$');
+// called for each blame entry, as soon as it finishes
+function handleLine(commit) {
+ /* This is the structure of the HTML fragment we are working
+ with:
+
+ <tr id="l123" class="">
+ <td class="sha1" title=""><a href=""></a></td>
+ <td class="linenr"><a class="linenr" href="">123</a></td>
+ <td class="pre"># times (my ext3 doesn't).</td>
+ </tr>
+ */
+
+ var resline = commit.resline;
+
+ if (!commit.info) {
+ var date = new Date();
+ date.setTime(commit.authorTime * 1000); // milliseconds
+ var dateStr =
+ date.getUTCFullYear() + '-'
+ + zeroPad(date.getUTCMonth()+1) + '-'
+ + zeroPad(date.getUTCDate());
+ var timeStr =
+ zeroPad(date.getUTCHours()) + ':'
+ + zeroPad(date.getUTCMinutes()) + ':'
+ + zeroPad(date.getUTCSeconds());
+
+ var localDate = new Date();
+ var match = tzRe.exec(commit.authorTimezone);
+ localDate.setTime(1000 * (commit.authorTime
+ + (parseInt(match[1],10)*3600 + parseInt(match[2],10)*60)));
+ var localTimeStr =
+ zeroPad(localDate.getUTCHours()) + ':'
+ + zeroPad(localDate.getUTCMinutes()) + ':'
+ + zeroPad(localDate.getUTCSeconds());
+
+ /* e.g. 'Kay Sievers, 2005-08-07 19:49:46 +0000 (21:49:46 +0200)' */
+ commit.info = commit.author + ', ' + dateStr + ' '
+ + timeStr + ' +0000'
+ + ' (' + localTimeStr + ' ' + commit.authorTimezone + ')';
+ }
+
+ var color = findColorNo(
+ document.getElementById('l'+(resline-1)),
+ document.getElementById('l'+(resline+commit.numlines))
+ );
+
+
+ for (var i = 0; i < commit.numlines; i++) {
+ var tr = document.getElementById('l'+resline);
+ if (!tr) {
+ debug('tr is null! resline: ' + resline);
+ break;
+ }
+ /*
+ <tr id="l123" class="">
+ <td class="sha1" title=""><a href=""></a></td>
+ <td class="linenr"><a class="linenr" href="">123</a></td>
+ <td class="pre"># times (my ext3 doesn't).</td>
+ </tr>
+ */
+ var td_sha1 = tr.firstChild;
+ var a_sha1 = td_sha1.firstChild;
+ var a_linenr = td_sha1.nextSibling.firstChild;
+
+ /* <tr id="l123" class=""> */
+ if (color) {
+ tr.setAttribute('class', 'color'+color.toString());
+ // Internet Explorer needs this
+ //tr.setAttribute('className', color.toString);
+ }
+ /* <td class="sha1" title="?" rowspan="?"><a href="?">?</a></td> */
+ if (i == 0) {
+ td_sha1.title = commit.info;
+ td_sha1.setAttribute('rowspan', commit.numlines);
+
+ a_sha1.href = baseUrl + '?a=commit;h=' + commit.sha1;
+ a_sha1.innerHTML = commit.sha1.substr(0, 8);
+ if (commit.numlines >= 2) {
+ var br = document.createElement("br");
+ var text = document.createTextNode(commit.author.match(/\b([A-Z])\B/g).join(''));
+ if (br && text) {
+ td_sha1.appendChild(br);
+ td_sha1.appendChild(text);
+ }
+ }
+ } else {
+ tr.removeChild(td_sha1);
+ }
+
+ /* <td class="linenr"><a class="linenr" href="?">123</a></td> */
+ a_linenr.href = baseUrl + '?a=blame;hb=' + commit.sha1
+ + ';f=' + commit.filename + '#l' + (commit.srcline + i);
+
+ resline++;
+ blamedLines++;
+
+ //updateProgressInfo();
+ }
+}
+
+function startOfGroup(tr) {
+ return tr.firstChild.getAttribute('class') == 'sha1';
+}
+
+function fixColors() {
+ var colorClasses = ['light2', 'dark2'];
+ var linenum = 1;
+ var tr;
+ var colorClass = 0;
+
+ while ((tr = document.getElementById('l'+linenum))) {
+ if (startOfGroup(tr, linenum, document)) {
+ colorClass = (colorClass + 1) % 2;
+ }
+ tr.setAttribute('class', colorClasses[colorClass]);
+ // Internet Explorer needs this
+ tr.setAttribute('className', colorClasses[colorClass]);
+ linenum++;
+ }
+}
+
+var t_interval_server = '';
+var t0 = new Date();
+
+function writeTimeInterval() {
+ var info = document.getElementById('generate_time');
+ if (!info)
+ return;
+ var t1 = new Date();
+
+ info.innerHTML += ' + '
+ + t_interval_server+'s server (blame_data) / '
+ + (t1.getTime() - t0.getTime())/1000 + 's client (JavaScript)';
+}
+
+// ----------------------------------------------------------------------
+
+var prevDataLength = -1;
+var nextLine = 0;
+var inProgress = false;
+
+var sha1Re = new RegExp('([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)');
+var infoRe = new RegExp('([a-z-]+) ?(.*)');
+var endRe = new RegExp('END ?(.*)');
+var curCommit = new Commit();
+
+var pollTimer = null;
+
+function handleResponse() {
+ debug('handleResp ready: ' + http.readyState
+ + ' respText null?: ' + (http.responseText === null)
+ + ' progress: ' + inProgress);
+
+ if (http.readyState != 4 && http.readyState != 3)
+ return;
+
+ // the server stream is incorrect
+ if (http.readyState == 3 && http.status != 200)
+ return;
+ if (http.readyState == 4 && http.status != 200) {
+ if (!div_progress_info)
+ div_progress_info = document.getElementById('progress_info');
+
+ if (div_progress_info) {
+ div_progress_info.setAttribute('class', 'error');
+ div_progress_info.innerHTML =
+ http.status+' - Error contacting server\n';
+ } else {
+ document.write("<b>ERROR:</b> HTTP status is "+http.status+"<br />\n");
+ }
+
+ clearInterval(pollTimer);
+ inProgress = false;
+ }
+
+ // In konqueror http.responseText is sometimes null here...
+ if (http.responseText === null)
+ return;
+
+ /*
+ var resp = document.getElementById('state');
+ if (resp) {
+ resp.innerHTML = http.readyState + ' : ' + http.status
+ + '<br />len = ' + http.responseText.length
+ + '; inProgress='+inProgress;
+ //inProgress = true;
+ }
+ */
+
+ if (inProgress)
+ return;
+ else
+ inProgress = true;
+
+ while (prevDataLength != http.responseText.length) {
+ if (http.readyState == 4
+ && prevDataLength == http.responseText.length) {
+ break;
+ }
+
+ prevDataLength = http.responseText.length;
+ var response = http.responseText.substring(nextLine);
+ var lines = response.split('\n');
+ nextLine = nextLine + response.lastIndexOf('\n') + 1;
+ if (response[response.length-1] != '\n') {
+ lines.pop();
+ }
+
+ for (var i = 0; i < lines.length; i++) {
+ var match = sha1Re.exec(lines[i]);
+ if (match) {
+ var sha1 = match[1];
+ var srcline = parseInt(match[2]);
+ var resline = parseInt(match[3]);
+ var numlines = parseInt(match[4]);
+ var c = commits[sha1];
+ if (!c) {
+ c = new Commit(sha1);
+ commits[sha1] = c;
+ }
+
+ c.srcline = srcline;
+ c.resline = resline;
+ c.numlines = numlines;
+ curCommit = c;
+ } else if ((match = infoRe.exec(lines[i]))) {
+ var info = match[1];
+ var data = match[2];
+ if (info == 'filename') {
+ curCommit.filename = data;
+ handleLine(curCommit);
+ updateProgressInfo();
+ } else if (info == 'author') {
+ curCommit.author = data;
+ } else if (info == 'author-time') {
+ curCommit.authorTime = parseInt(data);
+ } else if (info == 'author-tz') {
+ curCommit.authorTimezone = data;
+ }
+ } else if ((match = endRe.exec(lines[i]))) {
+ t_interval_server = match[1];
+ debug('END: '+lines[i]);
+ } else if (lines[i] != '') {
+ debug('malformed line: ' + lines[i]);
+ }
+ }
+ }
+
+ if (http.readyState == 4 &&
+ prevDataLength == http.responseText.length) {
+ clearInterval(pollTimer);
+
+ fixColors();
+ writeTimeInterval();
+ }
+
+ inProgress = false;
+}
+
+function startBlame(blamedataUrl, bUrl) {
+ debug('startBlame('+blamedataUrl+', '+bUrl+')');
+
+ t0 = new Date();
+ baseUrl = bUrl;
+ totalLines = countLines();
+ updateProgressInfo();
+
+ http = createRequestObject();
+ http.open('get', blamedataUrl);
+ http.onreadystatechange = handleResponse;
+ http.send(null);
+
+ pollTimer = setInterval(handleResponse, 1000);
+}
+
+// end of blame.js
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index a01eac8..e359618 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -223,11 +223,7 @@ tr.light:hover {
background-color: #edece6;
}
-tr.dark {
- background-color: #f6f6f0;
-}
-
-tr.dark2 {
+tr.dark, tr.dark2 {
background-color: #f6f6f0;
}
@@ -235,6 +231,14 @@ tr.dark:hover {
background-color: #edece6;
}
+tr.color1:hover { background-color: #e6ede6; }
+tr.color2:hover { background-color: #e6e6ed; }
+tr.color3:hover { background-color: #ede6e6; }
+
+tr.color1 { background-color: #f6fff6; }
+tr.color2 { background-color: #f6f6ff; }
+tr.color3 { background-color: #fff6f6; }
+
td {
padding: 2px 5px;
font-size: 100%;
@@ -255,7 +259,7 @@ td.sha1 {
font-family: monospace;
}
-td.error {
+.error {
color: red;
background-color: yellow;
}
@@ -326,6 +330,17 @@ td.mode {
font-family: monospace;
}
+/* progress of blame_interactive */
+div#progress_bar {
+ height: 2px;
+ margin-bottom: -2px;
+ background-color: #d8d9d0;
+}
+div#progress_info {
+ float: right;
+ text-align: right;
+}
+
/* styling of diffs (patchsets): commitdiff and blobdiff views */
div.diff.header,
div.diff.extended_header {
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4987fdc..774bcc6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,6 +18,9 @@ use File::Find qw();
use File::Basename qw(basename);
binmode STDOUT, ':utf8';
+use Time::HiRes qw(gettimeofday tv_interval);
+our $t0 = [gettimeofday];
+
BEGIN {
CGI->compile() if $ENV{'MOD_PERL'};
}
@@ -74,6 +77,8 @@ our $stylesheet = undef;
our $logo = "++GITWEB_LOGO++";
# URI of GIT favicon, assumed to be image/png type
our $favicon = "++GITWEB_FAVICON++";
+# URI of blame.js
+our $blamejs = "++GITWEB_BLAMEJS++";
# URI and label (title) of GIT logo link
#our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
@@ -493,6 +498,8 @@ our %cgi_param_mapping = @cgi_param_mapping;
# we will also need to know the possible actions, for validation
our %actions = (
"blame" => \&git_blame,
+ "blame_incremental" => \&git_blame_incremental,
+ "blame_data" => \&git_blame_data,
"blobdiff" => \&git_blobdiff,
"blobdiff_plain" => \&git_blobdiff_plain,
"blob" => \&git_blob,
@@ -2874,13 +2881,13 @@ sub git_header_html {
# 'application/xhtml+xml', otherwise send it as plain old 'text/html'.
# we have to do this because MSIE sometimes globs '*/*', pretending to
# support xhtml+xml but choking when it gets what it asked for.
- if (defined $cgi->http('HTTP_ACCEPT') &&
- $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
- $cgi->Accept('application/xhtml+xml') != 0) {
- $content_type = 'application/xhtml+xml';
- } else {
+ #if (defined $cgi->http('HTTP_ACCEPT') &&
+ # $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
+ # $cgi->Accept('application/xhtml+xml') != 0) {
+ # $content_type = 'application/xhtml+xml';
+ #} else {
$content_type = 'text/html';
- }
+ #}
print $cgi->header(-type=>$content_type, -charset => 'utf-8',
-status=> $status, -expires => $expires);
my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : '';
@@ -3042,6 +3049,14 @@ sub git_footer_html {
}
print "</div>\n"; # class="page_footer"
+ print "<div class=\"page_footer\">\n";
+ print 'This page took '.
+ '<span id="generate_time" class="time_span">'.
+ tv_interval($t0, [gettimeofday]).'s'.
+ '</span>'.
+ " to generate.\n";
+ print "</div>\n"; # class="page_footer"
+
if (-f $site_footer) {
insert_file($site_footer);
}
@@ -4574,7 +4589,9 @@ sub git_tag {
git_footer_html();
}
-sub git_blame {
+sub git_blame_common {
+ my $format = shift || 'porcelain';
+
# permissions
gitweb_check_feature('blame')
or die_error(403, "Blame view not allowed");
@@ -4596,10 +4613,36 @@ sub git_blame {
}
}
- # run git-blame --porcelain
- open my $fd, "-|", git_cmd(), "blame", '-p',
- $hash_base, '--', $file_name
- or die_error(500, "Open git-blame failed");
+ my $fd;
+ if ($format eq 'incremental') {
+ # get file contents (as base)
+ open $fd, "-|", git_cmd(), 'cat-file', 'blob', $hash
+ or die_error(500, "Open git-cat-file failed");
+ } elsif ($format eq 'data') {
+ # run git-blame --incremental
+ open $fd, "-|", git_cmd(), "blame", "--incremental",
+ $hash_base, "--", $file_name
+ or die_error(500, "Open git-blame --incremental failed");
+ } else {
+ # run git-blame --porcelain
+ open $fd, "-|", git_cmd(), "blame", '-p',
+ $hash_base, '--', $file_name
+ or die_error(500, "Open git-blame --porcelain failed");
+ }
+
+ # incremental blame data returns early
+ if ($format eq 'data') {
+ print $cgi->header(
+ -type=>"text/plain", -charset => "utf-8",
+ -status=> "200 OK");
+ local $| = 1; # output autoflush
+ print while <$fd>;
+ close $fd
+ or print "ERROR $!\n";
+ print "END ".tv_interval($t0, [gettimeofday])."\n";
+
+ return;
+ }
# page header
git_header_html();
@@ -4610,93 +4653,134 @@ sub git_blame {
$cgi->a({-href => href(action=>"history", -replay=>1)},
"history") .
" | " .
- $cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
+ $cgi->a({-href => href(action=>$action, file_name=>$file_name)},
"HEAD");
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
git_print_page_path($file_name, $ftype, $hash_base);
# page body
+ print qq!<div id="progress_bar" style="width: 100%; background-color: yellow"></div>\n!
+ if ($format eq 'incremental');
+ print qq!<div class="page_body">\n!;
+ print qq!<div id="progress_info">... / ...</div>\n!
+ if ($format eq 'incremental');
+ print qq!<table id="blame_table" class="blame" width="100%">\n!.
+ #qq!<col width="5.5em" /><col width="2.5em" /><col width="*" />\n!.
+ qq!<tr><th>Commit</th><th>Line</th><th>Data</th></tr>\n!;
+
my @rev_color = qw(light2 dark2);
my $num_colors = scalar(@rev_color);
my $current_color = 0;
- my %metainfo = ();
- print <<HTML;
-<div class="page_body">
-<table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
-HTML
- LINE:
- while (my $line = <$fd>) {
- chomp $line;
- # the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
- # no <lines in group> for subsequent lines in group of lines
- my ($full_rev, $orig_lineno, $lineno, $group_size) =
- ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
- if (!exists $metainfo{$full_rev}) {
- $metainfo{$full_rev} = {};
- }
- my $meta = $metainfo{$full_rev};
- my $data; # last line is used later
- while ($data = <$fd>) {
- chomp $data;
- last if ($data =~ s/^\t//); # contents of line
- if ($data =~ /^(\S+) (.*)$/) {
- $meta->{$1} = $2;
- }
- }
- my $short_rev = substr($full_rev, 0, 8);
- my $author = $meta->{'author'};
- my %date =
- parse_date($meta->{'author-time'}, $meta->{'author-tz'});
- my $date = $date{'iso-tz'};
- if ($group_size) {
- $current_color = ($current_color + 1) % $num_colors;
- }
- print "<tr id=\"l$lineno\" class=\"$rev_color[$current_color]\">\n";
- if ($group_size) {
- print "<td class=\"sha1\"";
- print " title=\"". esc_html($author) . ", $date\"";
- print " rowspan=\"$group_size\"" if ($group_size > 1);
- print ">";
- print $cgi->a({-href => href(action=>"commit",
- hash=>$full_rev,
- file_name=>$file_name)},
- esc_html($short_rev));
- print "</td>\n";
+ if ($format eq 'incremental') {
+ my $color_class = $rev_color[$current_color];
+
+ #contents of a file
+ my $linenr = 0;
+ LINE:
+ while (my $line = <$fd>) {
+ chomp $line;
+ $linenr++;
+
+ print qq!<tr id="l$linenr" class="$color_class">!.
+ qq!<td class="sha1"><a href=""></a></td>!.
+ qq!<td class="linenr">!.
+ qq!<a class="linenr" href="">$linenr</a></td>!;
+ print qq!<td class="pre">! . esc_html($line) . "</td>\n";
+ print qq!</tr>\n!;
}
- my $parent_commit;
- if (!exists $meta->{'parent'}) {
- open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
- or die_error(500, "Open git-rev-parse failed");
- $parent_commit = <$dd>;
- close $dd;
- chomp($parent_commit);
- $meta->{'parent'} = $parent_commit;
- } else {
- $parent_commit = $meta->{'parent'};
- }
- my $blamed = href(action => 'blame',
- file_name => $meta->{'filename'},
- hash_base => $parent_commit);
- print "<td class=\"linenr\">";
- print $cgi->a({ -href => "$blamed#l$orig_lineno",
- -class => "linenr" },
- esc_html($lineno));
- print "</td>";
- print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
- print "</tr>\n";
- }
- print "</table>\n";
- print "</div>";
+
+ } else { # porcelain, i.e. ordinary blame
+ my %metainfo = (); # saves information about commits
+
+ # blame data
+ LINE:
+ while (my $line = <$fd>) {
+ chomp $line;
+ # the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
+ # no <lines in group> for subsequent lines in group of lines
+ my ($full_rev, $orig_lineno, $lineno, $group_size) =
+ ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
+ $metainfo{$full_rev} ||= {};
+ my $meta = $metainfo{$full_rev};
+ my $data; # last line is used later
+ while ($data = <$fd>) {
+ chomp $data;
+ last if ($data =~ s/^\t//); # contents of line
+ if ($data =~ /^(\S+) (.*)$/) {
+ $meta->{$1} = $2;
+ }
+ }
+ my $short_rev = substr($full_rev, 0, 8);
+ my $author = $meta->{'author'};
+ my %date =
+ parse_date($meta->{'author-time'}, $meta->{'author-tz'});
+ my $date = $date{'iso-tz'};
+ if ($group_size) {
+ $current_color = ($current_color + 1) % $num_colors;
+ }
+ print qq!<tr id="l$lineno" class="$rev_color[$current_color]">\n!;
+ if ($group_size) {
+ print qq!<td class="sha1"!.
+ qq! title="!. esc_html($author) . qq!, $date"!;
+ print qq! rowspan="$group_size"! if ($group_size > 1);
+ print qq!>!;
+ print $cgi->a({-href => href(action=>"commit",
+ hash=>$full_rev,
+ file_name=>$file_name)},
+ esc_html($short_rev));
+ print qq!</td>\n!;
+ }
+ if (!exists $meta->{'parent'}) {
+ open my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^"
+ or die_error(500, "Open git-rev-parse failed");
+ $meta->{'parent'} = <$dd>;
+ close $dd;
+ chomp $meta->{'parent'};
+ }
+ my $blamed = href(action => 'blame',
+ file_name => $meta->{'filename'},
+ hash_base => $meta->{'parent'});
+ print qq!<td class="linenr">!.
+ $cgi->a({ -href => "$blamed#l$orig_lineno",
+ -class => "linenr" },
+ esc_html($lineno)).
+ qq!</td>!;
+ print qq!<td class="pre">! . esc_html($data) . "</td>\n";
+ print qq!</tr>\n!;
+ }
+ }
+
+ # footer
+ print "</table>\n"; # class="blame"
+ print "</div>\n"; # class="blame_body"
close $fd
or print "Reading blob failed\n";
- # page footer
+ if ($format eq 'incremental') {
+ print qq!<script type="text/javascript" src="$blamejs"></script>\n!.
+ qq!<script type="text/javascript">\n!.
+ qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
+ qq! "$my_url");\n!.
+ qq!</script>\n!;
+ }
+
git_footer_html();
}
+sub git_blame {
+ git_blame_common();
+}
+
+sub git_blame_incremental {
+ git_blame_common('incremental');
+}
+
+sub git_blame_data {
+ git_blame_common('data');
+}
+
sub git_tags {
my $head = git_get_head_hash($project);
git_header_html();
--
Stacked GIT 0.14.3
git version 1.6.0.4
^ permalink raw reply related
* Re: fatal output from git-show really wants a terminal
From: Tim Olsen @ 2008-12-10 20:10 UTC (permalink / raw)
To: git; +Cc: git
In-Reply-To: <200812102046.50186.j6t@kdbg.org>
Johannes Sixt wrote:
> The pattern is that if stdout is a terminal, the pager is thrown up and both
> stdout and stderr of git show proper are redirected to the pager. If you
> redirect only stderr, then this redirection is actually ignored.
I see. So I probably want to use --no-pager.
Thanks for the help.
-Tim
>
> -- Hannes
^ permalink raw reply
* Re: builtin-add.c patch
From: Alexander Potashev @ 2008-12-10 20:10 UTC (permalink / raw)
To: root; +Cc: git
In-Reply-To: <200812101914.mBAJEAS04718@localhost.localdomain>
Hello, Tim!
On 14:14 Wed 10 Dec , root wrote:
> Alexander,
>
> I saw a suggestion that git could be used as a filesystem rather
> than as a code repository. I'm looking to convert it for this
> purpose to sit underneath Axiom, a computer algebra system written
> in common lisp. Basically the idea is that a "close" operation does
> a 'git add foo ; git commit'.
>
> Are you aware of anyone who has used git as a filesystem?
>
> Tim Daly
>
It's a quite off-topic question. But Git is not optimized to track
individual files with separate history
( see http://www.youtube.com/watch?v=8dhZ9BXQgc4 ).
Also, Git uses only 644 and 755 permissions (755 stands for
executables, often scripts - shell scripts, perl, ...), but
usual filesystems provide full range of premissions/ownership.
Alexander
^ permalink raw reply
* Re: [PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()
From: Luben Tuikov @ 2008-12-10 20:05 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200812101615.55340.jnareb@gmail.com>
--- On Wed, 12/10/08, Jakub Narebski <jnareb@gmail.com> wrote:
> > Have you tested this patch that it gives the same
> commit chain
> > as before it?
>
> The only difference between precious version and this patch
> is that
> now, if you calculate sha-1 of $long_rev^, it is stored in
> $metainfo{$long_rev}{'parent'} and not calculated
> second time.
Yes, I agree a patch to this effect would improve performance
proportionally to the history of the lines of a file. So it's a
good thing, as most commits change a contiguous block of size more
than one line of a file.
"$parent_commit" depends on "$full_rev^" which depends on "$full_rev".
So as soon as "$full_rev" != "$old_full_rev", you'd know that you need
to update "$parent_commit". "$old_full_rev" needs to exist outside the
scope of "while (1)". I didn't see this in the code or in the patch.
> But I have checked that (at least for single example file)
> the blame output is identical for before and after this patch.
I've always tested it on something like "gitweb.perl", etc.
Luben
^ permalink raw reply
* Re: [PATCH 2/2] bash completion: Use 'git add' completions for 'git stage'
From: Shawn O. Pearce @ 2008-12-10 19:59 UTC (permalink / raw)
To: Lee Marlow; +Cc: git
In-Reply-To: <1228937958-5091-2-git-send-email-lee.marlow@gmail.com>
Lee Marlow <lee.marlow@gmail.com> wrote:
> Signed-off-by: Lee Marlow <lee.marlow@gmail.com>
> ---
> contrib/completion/git-completion.bash | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
Also,
Trivially-Acked-by: Shawn O. Pearce <spearce@spearce.org>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5356e5b..7e2b482 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1660,6 +1660,7 @@ _git ()
> show) _git_show ;;
> show-branch) _git_show_branch ;;
> stash) _git_stash ;;
> + stage) _git_add ;;
> submodule) _git_submodule ;;
> svn) _git_svn ;;
> tag) _git_tag ;;
> --
> 1.6.1.rc2.14.g5363d
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Shawn.
^ permalink raw reply
* Re: fatal output from git-show really wants a terminal
From: Johannes Sixt @ 2008-12-10 19:46 UTC (permalink / raw)
To: Tim Olsen; +Cc: git
In-Reply-To: <ghop5d$qud$1@ger.gmane.org>
On Mittwoch, 10. Dezember 2008, Tim Olsen wrote:
> It appears that when outputting a fatal error, git-show will choose
> stdout over stderr if stdout is a terminal and stderr is not.
This is by design.
> How do I
> redirect the error but still allow stdout to be displayed?
$ git show 12345 2> /dev/null | less
> ~/git$ mkdir test
> ~/git$ cd test
> ~/git/test$ git init
> ~/git/test$ git show 12345
> fatal: ambiguous argument '12345': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions
You see this through the pager.
> ~/git/test$ git show 12345 2> /dev/null
> fatal: ambiguous argument '12345': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions
And this went through the pager as well.
> ~/git/test$ git show 12345 > /dev/null
> fatal: ambiguous argument '12345': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions
This went straight to the terminal.
The pattern is that if stdout is a terminal, the pager is thrown up and both
stdout and stderr of git show proper are redirected to the pager. If you
redirect only stderr, then this redirection is actually ignored.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/2] bash completion: Add '--intent-to-add' long option for 'git add'
From: Shawn O. Pearce @ 2008-12-10 19:41 UTC (permalink / raw)
To: Lee Marlow; +Cc: git
In-Reply-To: <1228937958-5091-1-git-send-email-lee.marlow@gmail.com>
Lee Marlow <lee.marlow@gmail.com> wrote:
> Signed-off-by: Lee Marlow <lee.marlow@gmail.com>
> ---
> contrib/completion/git-completion.bash | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Trivially-Acked-by: Shawn O. Pearce <spearce@spearce.org>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c79c98f..5356e5b 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -563,7 +563,7 @@ _git_add ()
> --*)
> __gitcomp "
> --interactive --refresh --patch --update --dry-run
> - --ignore-errors
> + --ignore-errors --intent-to-add
> "
> return
> esac
> --
> 1.6.1.rc2.14.g5363d
>
--
Shawn.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox