* Re: git-svn of both trunk and tags while having no access to the 'parent' of those
From: Michael J Gruber @ 2009-11-18 15:07 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git, Eric Wong
In-Reply-To: <20091118142332.GC17964@onerussian.com>
Yaroslav Halchenko venit, vidit, dixit 18.11.2009 15:23:
>
> On Wed, 18 Nov 2009, Michael J Gruber wrote:
>>> git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo/trunk svnrepo.gitsvn
>> Is the trunk really at svnrepo/trunk/trunk?
> nope... it is just svnrepo/trunk but if I set url to point to parent --
> git svn seeks authentication right away
>
>> I would try both
>> git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t
>> releases http://domain.com/svnrepo/ svnrepo.gitsvn
>
> asks for authentication since there is no public access to
> http://domain.com/svnrepo/
>
>> and also the seemingly equivalent
>
>> git svn clone --no-minimize-url --prefix=upstream-svn/ -T
>> http://domain.com/svnrepo/trunk -t http://domain.com/svnrepo/releases
>> svnrepo.gitsvn
> seems to not work since it wants url as a parameter
>
> Bad URL passed to RA layer: Illegal repository URL svnrepo.gitsvn at /usr/lib/git-core/git-svn line 940
>
>> Also, I assume you can svn list http://domain.com/svnrepo/trunk and
>> http://domain.com/svnrepo/releases ;)
> yeap -- I can list both of those but not their parent.
>
>
OK, so the way it's implemented --no-minimize-url might be half as
useful as it could be. One last try (before asking Eric...) would be
git svn clone --no-minimize-url --prefix=upstream-svn/ -T
http://domain.com/svnrepo/trunk -t http://domain.com/svnrepo/releases
http://domain.com/svnrepo/trunk svnrepo.gitsvn
because that involves accessible URLs only and trunk and branch URLs are
absolute.
[Meanwhile I git the actual URL PMed by Yaroslov.]
So, what happens with the above is that git-svn tries to set the URL
config again from the URL part of an absolute tags argument. I don't
know how absolute URLs (which are documented) for tags etc. could
possibly work if git-svn tries to do that. Eric?
I tried also with two svn sections to circumvent this:
[svn-remote "svn"]
url = http://domain.com:/project/trunk
fetch = :refs/remotes/trunk
[svn-remote "svnr"]
url = http://domain.com:/project/releases
tags = /*:refs/remotes/tags/*
Fetching -Rsvn works fine, but fetching -Rsvnr gives the same
authentication problems. And fetch does not accept --no-minimize-url as
an option. OTOH: If the option is not used it seems to me (from the
source) that not minimizing is the default, which leaves me really
stomped. Eric?? ;)
Michael
^ permalink raw reply
* [PATCH v2] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-18 15:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Bert Wesarg
In-Reply-To: <1258543836-799-1-git-send-email-bert.wesarg@googlemail.com>
Inspired by the coloring of quilt.
Introduce a separate color for the hunk comment part, i.e. the current function.
Whitespace between hunk header and hunk comment is now printed as plain.
The current default is magenta. But I'm not settled on this. My favorite would
be bold yellow.
Now with updated test suit.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
Documentation/config.txt | 8 +++---
combine-diff.c | 5 +++-
diff.c | 64 +++++++++++++++++++++++++++++++++++++++++++--
diff.h | 1 +
t/t4034-diff-words.sh | 3 +-
5 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index cb73d75..421cd50 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -598,10 +598,10 @@ color.diff.<slot>::
Use customized color for diff colorization. `<slot>` specifies
which part of the patch to use the specified color, and is one
of `plain` (context text), `meta` (metainformation), `frag`
- (hunk header), `old` (removed lines), `new` (added lines),
- `commit` (commit headers), or `whitespace` (highlighting
- whitespace errors). The values of these variables may be specified as
- in color.branch.<slot>.
+ (hunk header), 'func' (function in hunk header), `old` (removed lines),
+ `new` (added lines), `commit` (commit headers), or `whitespace`
+ (highlighting whitespace errors). The values of these variables may be
+ specified as in color.branch.<slot>.
color.grep::
When set to `always`, always highlight matches. When `false` (or
diff --git a/combine-diff.c b/combine-diff.c
index 5b63af1..6162691 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -524,6 +524,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
int i;
unsigned long lno = 0;
const char *c_frag = diff_get_color(use_color, DIFF_FRAGINFO);
+ const char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);
const char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);
const char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);
const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
@@ -588,7 +589,9 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
comment_end = i;
}
if (comment_end)
- putchar(' ');
+ printf("%s%s %s%s", c_reset,
+ c_plain, c_reset,
+ c_func);
for (i = 0; i < comment_end; i++)
putchar(hunk_comment[i]);
}
diff --git a/diff.c b/diff.c
index 0d7f5ea..e210525 100644
--- a/diff.c
+++ b/diff.c
@@ -39,6 +39,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_GREEN, /* NEW */
GIT_COLOR_YELLOW, /* COMMIT */
GIT_COLOR_BG_RED, /* WHITESPACE */
+ GIT_COLOR_MAGENTA, /* FUNCINFO */
};
static void diff_filespec_load_driver(struct diff_filespec *one);
@@ -60,6 +61,8 @@ static int parse_diff_color_slot(const char *var, int ofs)
return DIFF_COMMIT;
if (!strcasecmp(var+ofs, "whitespace"))
return DIFF_WHITESPACE;
+ if (!strcasecmp(var+ofs, "func"))
+ return DIFF_FUNCINFO;
die("bad config variable '%s'", var);
}
@@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,
}
}
+static void emit_hunk_line(struct emit_callback *ecbdata,
+ const char *line, int len)
+{
+ const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
+ const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
+ const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
+ const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
+ const char *orig_line = line;
+ int orig_len = len;
+ const char *frag_start;
+ int frag_len;
+ const char *part_end = NULL;
+ int part_len = 0;
+
+ /* determine length of @ */
+ while (part_len < len && line[part_len] == '@')
+ part_len++;
+
+ /* find end of frag, (Ie. find second @@) */
+ part_end = memmem(line + part_len, len - part_len,
+ line, part_len);
+ if (!part_end)
+ return emit_line(ecbdata->file, frag, reset, line, len);
+ /* calculate total length of frag */
+ part_len = (part_end + part_len) - line;
+
+ /* remember frag part, we emit only if we find a space separator */
+ frag_start = line;
+ frag_len = part_len;
+
+ /* consume hunk header */
+ len -= part_len;
+ line += part_len;
+
+ /*
+ * for empty reminder or empty space sequence (exclusive any newlines
+ * or carriage returns) emit complete original line as FRAGINFO
+ */
+ if (!len || !(part_len = strspn(line, " \t")))
+ return emit_line(ecbdata->file, frag, reset,
+ orig_line, orig_len);
+
+ /* now emit the hunk header as FRAGINFO */
+ emit_line(ecbdata->file, frag, reset, frag_start, frag_len);
+
+ /* print whitespace sep as PLAIN */
+ emit_line(ecbdata->file, plain, reset, line, part_len);
+
+ /* consume whitespace sep */
+ len -= part_len;
+ line += part_len;
+
+ /* print reminder as FUNCINFO */
+ if (len)
+ emit_line(ecbdata->file, func, reset, line, len);
+}
+
static struct diff_tempfile *claim_diff_tempfile(void) {
int i;
for (i = 0; i < ARRAY_SIZE(diff_temp); i++)
@@ -781,9 +841,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
diff_words_flush(ecbdata);
len = sane_truncate_line(ecbdata, line, len);
find_lno(line, ecbdata);
- emit_line(ecbdata->file,
- diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
- reset, line, len);
+ emit_hunk_line(ecbdata, line, len);
if (line[len-1] != '\n')
putc('\n', ecbdata->file);
return;
diff --git a/diff.h b/diff.h
index 2740421..15fcecd 100644
--- a/diff.h
+++ b/diff.h
@@ -130,6 +130,7 @@ enum color_diff {
DIFF_FILE_NEW = 5,
DIFF_COMMIT = 6,
DIFF_WHITESPACE = 7,
+ DIFF_FUNCINFO = 8,
};
const char *diff_get_color(int diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 21db6e9..64a7c38 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -16,6 +16,7 @@ decrypt_color () {
-e 's/.\[1m/<WHITE>/g' \
-e 's/.\[31m/<RED>/g' \
-e 's/.\[32m/<GREEN>/g' \
+ -e 's/.\[35m/<MAGENTA>/g' \
-e 's/.\[36m/<BROWN>/g' \
-e 's/.\[m/<RESET>/g'
}
@@ -70,7 +71,7 @@ cat > expect <<\EOF
<WHITE>+++ b/post<RESET>
<BROWN>@@ -1 +1 @@<RESET>
<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
-<BROWN>@@ -3,0 +4,4 @@ a = b + c<RESET>
+<BROWN>@@ -3,0 +4,4 @@<RESET> <RESET><MAGENTA>a = b + c<RESET>
<GREEN>aa = a<RESET>
--
tg: (785c58e..) bw/func-color (depends on: master)
^ permalink raw reply related
* Re: [PATCH] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-18 15:16 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091118142320.GA1220@coredump.intra.peff.net>
On Wed, Nov 18, 2009 at 15:23, Jeff King <peff@peff.net> wrote:
> PS I almost complained about your default of "magenta" as the same as
> the meta color before I remembered that magenta meta is a personal
> setting I use. Personally I find the bold meta color to be distractingly
> ugly. Blaming it, the default seems to come from Linus, who even in his
> commit message (50f575f) seems to indicate that it is somewhat arbitrary
> (mostly just dropping the purple from the bold purple).
I took magenta, because it is not used as any other default color
value. I think choosing a color other than cyan would bring the
attention to this new feature.
Bert
^ permalink raw reply
* Re: [PATCH v2] Give the hunk comment its own color
From: Jason Sewall @ 2009-11-18 15:17 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Junio C Hamano, git
In-Reply-To: <1258557087-31540-1-git-send-email-bert.wesarg@googlemail.com>
On Wed, Nov 18, 2009 at 10:11 AM, Bert Wesarg
<bert.wesarg@googlemail.com> wrote:
> Now with updated test suit.
Spelling this as 'suite' would be sweet.
^ permalink raw reply
* Re: [PATCH v2] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-18 15:20 UTC (permalink / raw)
To: Jason Sewall; +Cc: Junio C Hamano, git
In-Reply-To: <31e9dd080911180717i27c6ef3fp736b7f8d41e4c8be@mail.gmail.com>
On Wed, Nov 18, 2009 at 16:17, Jason Sewall <jasonsewall@gmail.com> wrote:
> On Wed, Nov 18, 2009 at 10:11 AM, Bert Wesarg
> <bert.wesarg@googlemail.com> wrote:
>
>> Now with updated test suit.
>
> Spelling this as 'suite' would be sweet.
Thanks. I re-submit only if you find a bug too.
Bert
>
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 15:51 UTC (permalink / raw)
To: Philip Hofstetter; +Cc: git
In-Reply-To: <aa2993680911180620g151d8a07t11144d150cd6e29e@mail.gmail.com>
On Wed, Nov 18, 2009 at 03:20:48PM +0100, Philip Hofstetter wrote:
> Some investigating revealed an interesting quirk in git-mailinfo which
> seems to be a bit too eager to extract author information: Instead of
> just looking at the From:-Line in a mails header (git-rebase seems to
> use git-am which in turn uses git-mailinfo), it searches for "from:"
> *anywhere* in the mail and uses the last found information as the
> source for the author information.
It is not quite "anywhere"; extra headers are respected at the very top
of the message body. This is intentional, to allow one to indicate that
a patch you are sending was authored by somebody else.
So the problem is slightly less severe; the body of your commit message
has to _start_ with "From:". Still, it is awfully ugly to hit a parsing
ambiguity like this when you are trying to do something as simple as
rebase.
Some solutions I can think of are:
1. Improve the header-finding heuristic to actually look for something
more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
head which other headers we handle in this position. Probably
Date, too).
2. Give mailinfo a "--strict" mode to indicate that it is directly
parsing the output of format-patch, and not some random email. Use
--strict when invoking "git am" via "git rebase".
> While I know it's rude to have a line beginning with "from:" (and it's
> even ruder to have a line beginning with "from "), IMHO the header
> ends at the first blank line and I see no reason to extract author
> information past the header.
As I explained above, there is a reason, but I don't think it's rude to
have either of those lines. You were, after all, writing a commit
message, not an email (and even if you were, it is a failure of the
storage format if it can't represent your data correctly). So I think
git is to blame here.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #04; Tue, 17)
From: Johannes Sixt @ 2009-11-18 16:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <fcaeb9bf0911180643w5e659340jd845aa202e6feca3@mail.gmail.com>
Nguyen Thai Ngoc Duy schrieb:
> On 11/18/09, Junio C Hamano <gitster@pobox.com> wrote:
>> * nd/sparse (2009-08-20) 19 commits.
>>
>> The latest update I didn't look at very closely but I had an impression
>> that it was touching very generic codepath that would affect non sparse
>> cases, iow the patch looked very scary (the entire series already is).
>
> I wonder if there is any other approach for sparse checkout? I'll see
> if I can improve it, but with a series touching unpack logic, diff
> core, .gitattributes/.gitignore, it's hard to get it right and
> obvious.
Just FYI: I run some of my installations with this series, but without
using sparse checkout to see if there are regressions. Nothing came up so
far.
-- Hannes
^ permalink raw reply
* [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Bert Wesarg @ 2009-11-18 16:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Bert Wesarg
I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this
the test t4252-am-options.sh fails because it calls grep with a .rej file:
grep "@@ -1,3 +1,3 @@" file-2.rej
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
t/test-lib.sh | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..6ac8dc6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -65,6 +65,8 @@ GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# CDPATH into the environment
unset CDPATH
+unset GREP_OPTIONS
+
case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
1|2|true)
echo "* warning: Some tests will not work if GIT_TRACE" \
--
tg: (785c58e..) bw/unset-GREP_OPTIONS (depends on: master)
^ permalink raw reply related
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 16:42 UTC (permalink / raw)
To: Philip Hofstetter; +Cc: git
In-Reply-To: <20091118155154.GA15184@coredump.intra.peff.net>
On Wed, Nov 18, 2009 at 10:51:54AM -0500, Jeff King wrote:
> So the problem is slightly less severe; the body of your commit message
> has to _start_ with "From:". Still, it is awfully ugly to hit a parsing
> ambiguity like this when you are trying to do something as simple as
> rebase.
>
> Some solutions I can think of are:
>
> 1. Improve the header-finding heuristic to actually look for something
> more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
> head which other headers we handle in this position. Probably
> Date, too).
>
> 2. Give mailinfo a "--strict" mode to indicate that it is directly
> parsing the output of format-patch, and not some random email. Use
> --strict when invoking "git am" via "git rebase".
Solution (2) seemed like a lot of work, so here is the relatively small
solution (1). I think looking for <.*@.*> is too restrictive, as people
may be using:
From: bare@example.com
which should remain valid. I just look for an '@' instead.
Note that this validation also applies to actual headers. Should we turn
it off for them? As it is, it breaks t3400, which uses a bogus email
address. I suppose we should probably preserve such bogosities if they
are in the official headers.
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index c90cd31..6d69ef3 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -275,6 +275,17 @@ static inline int cmp_header(const struct strbuf *line, const char *hdr)
line->buf[len] == ':' && isspace(line->buf[len + 1]);
}
+static int validate_header(const char *header, const struct strbuf *data)
+{
+ if (!strcmp(header, "From"))
+ return !!strchr(data->buf, '@');
+ if (!strcmp(header, "Date")) {
+ char buf[50];
+ return parse_date(data->buf, buf, sizeof(buf)) >= 0;
+ }
+ return 1;
+}
+
static int check_header(const struct strbuf *line,
struct strbuf *hdr_data[], int overwrite)
{
@@ -289,8 +300,10 @@ static int check_header(const struct strbuf *line,
*/
strbuf_add(&sb, line->buf + len + 2, line->len - len - 2);
decode_header(&sb);
- handle_header(&hdr_data[i], &sb);
- ret = 1;
+ if (validate_header(header[i], &sb)) {
+ ret = 1;
+ handle_header(&hdr_data[i], &sb);
+ }
goto check_header_out;
}
}
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 0279d07..be06e0f 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
- test `cat last` = 14'
+ test `cat last` = 16'
check_mailinfo () {
mail=$1 opt=$2
diff --git a/t/t5100/info0015 b/t/t5100/info0015
new file mode 100644
index 0000000..c4d8d77
--- /dev/null
+++ b/t/t5100/info0015
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0016 b/t/t5100/info0016
new file mode 100644
index 0000000..f4857d4
--- /dev/null
+++ b/t/t5100/info0016
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
new file mode 100644
index 0000000..be5115b
--- /dev/null
+++ b/t/t5100/msg0015
@@ -0,0 +1,3 @@
+From: bogosity
+ - a list
+ - of stuff
diff --git a/t/t5100/msg0016 b/t/t5100/msg0016
new file mode 100644
index 0000000..1063f51
--- /dev/null
+++ b/t/t5100/msg0016
@@ -0,0 +1,4 @@
+Date: bogus
+
+and some content
+
diff --git a/t/t5100/patch0015 b/t/t5100/patch0015
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016 b/t/t5100/patch0016
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 13fa4ae..de10312 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -650,3 +650,36 @@ index b0b5d8f..461c47e 100644
convert_to_utf8(line, charset.buf);
--
1.6.4.1
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: bogosity
+ - a list
+ - of stuff
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+Date: bogus
+
+and some content
+
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
^ permalink raw reply related
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Philip Hofstetter @ 2009-11-18 17:11 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091118155154.GA15184@coredump.intra.peff.net>
Hello,
On Wed, Nov 18, 2009 at 4:51 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 18, 2009 at 03:20:48PM +0100, Philip Hofstetter wrote:
> 1. Improve the header-finding heuristic to actually look for something
> more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
> head which other headers we handle in this position. Probably
> Date, too).
or at least don't prefer obviously invalid data over valid data that
has already been seen.
> 2. Give mailinfo a "--strict" mode to indicate that it is directly
> parsing the output of format-patch, and not some random email. Use
> --strict when invoking "git am" via "git rebase".
That would solve the problem too, though it feels like adding yet
another switch to guard against one specific issue. The purpose behind
options like this tends to get forgotten over time.
> As I explained above, there is a reason, but I don't think it's rude to
> have either of those lines. You were, after all, writing a commit
> message, not an email (and even if you were, it is a failure of the
> storage format if it can't represent your data correctly). So I think
> git is to blame here.
IMHO, another workable solution would be to reject a commit that later
can't be handled. That way the current attempts at getting an email
address can remain intact and the (much more) unlikely case that
somebody begins the commit message with from: will be caught before
damage is done.
So, just check that from-line for a valid email address at commit
time. If it is, ok. If not, treat it as an error and inform the user
that an invalid email address was given in the commit message.
Also, the error message by rebase (which is actually the message
printed by am) could have been a bit more helpful. If am fails during
a rebase, rebase could explicitly tell which commit am failed at. The
output I got made me suspect the problem to be in the first commit (as
that was the last one printed) when in fact it was in the second one
(which was not printed).
But that's just nit-picking.
Philip
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 17:24 UTC (permalink / raw)
To: Philip Hofstetter; +Cc: git
In-Reply-To: <aa2993680911180911o7e3af804m4ebdc20096baa609@mail.gmail.com>
On Wed, Nov 18, 2009 at 06:11:36PM +0100, Philip Hofstetter wrote:
> > As I explained above, there is a reason, but I don't think it's rude to
> > have either of those lines. You were, after all, writing a commit
> > message, not an email (and even if you were, it is a failure of the
> > storage format if it can't represent your data correctly). So I think
> > git is to blame here.
>
> IMHO, another workable solution would be to reject a commit that later
> can't be handled. That way the current attempts at getting an email
> address can remain intact and the (much more) unlikely case that
> somebody begins the commit message with from: will be caught before
> damage is done.
I'm not sure I like that solution for a few reasons:
1. It creates a bad user experience. You are not unreasonable for
wanting to put some specific text in your commit message. Having
git come back and say "oops, I might get confused by this later"
just seems like an annoyance to the user.
2. Mailinfo has to deal with data created by older versions of git. So
in your case, the rebase was a bomb waiting to go off. If we can
fix it so that an existing bomb doesn't go off, rather than not
creating the bomb in the first place, then we are better off.
3. Commit has to know about rules for mailinfo, even versions of
mailinfo that will exist in the future. Probably the rules aren't
going to change much, but it is a weakness.
4. Commit messages can come from other places than "git commit". What
should we do with a commit message like this that is imported from
SVN? Reject the import? Munge the message?
Of course all of that presupposes that we can correctly handle the
existing data after the fact. Even with my patch, you still can't write
"From: foo@example.com" as the first line of your commit body. But that
is, IMHO, getting even more unlikely than your "From:" (which already
seems fairly unlikely).
I also think "git commit" would not be the right time for such a
feature. The problem is not that you have this text in your commit
message. The problem is that the "format-patch | am" transport is lossy.
You would do better to have format-patch say "Ah, this is going to
create a bogus email address" and somehow quote it appropriately.
-Peff
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jakub Narebski @ 2009-11-18 17:46 UTC (permalink / raw)
To: Jeff King; +Cc: Philip Hofstetter, git
In-Reply-To: <20091118172424.GA24416@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I also think "git commit" would not be the right time for such a
> feature. The problem is not that you have this text in your commit
> message. The problem is that the "format-patch | am" transport is lossy.
> You would do better to have format-patch say "Ah, this is going to
> create a bogus email address" and somehow quote it appropriately.
Doesn't mbox format have some way of escaping "From:" (or is it "From ")?
If I remember correctly it uses ">From " or something for that.
git-format-patch could do this also (perhaps only with --rebasing
option).
P.S. As git-format-patch / git-am have hidden --rebasing option,
perhaps git-mailinfo should have it as well (even if it is called
--strict).
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 18:42 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Philip Hofstetter, git
In-Reply-To: <m3bpizd8ua.fsf@localhost.localdomain>
On Wed, Nov 18, 2009 at 09:46:43AM -0800, Jakub Narebski wrote:
> > I also think "git commit" would not be the right time for such a
> > feature. The problem is not that you have this text in your commit
> > message. The problem is that the "format-patch | am" transport is lossy.
> > You would do better to have format-patch say "Ah, this is going to
> > create a bogus email address" and somehow quote it appropriately.
>
> Doesn't mbox format have some way of escaping "From:" (or is it "From ")?
> If I remember correctly it uses ">From " or something for that.
> git-format-patch could do this also (perhaps only with --rebasing
> option).
It's for "From " lines, which are the mbox separator. This would be
somewhat different. It has nothing at all to do with the mail format
itself, but rather is about git treating the body of the message
specially. I don't think a quoting mechanism currently exists to handle
this.
-Peff
^ permalink raw reply
* RE: Hey - A Conceptual Simplication....
From: George Dennie @ 2009-11-18 18:51 UTC (permalink / raw)
To: 'Jan Krüger'; +Cc: git
In-Reply-To: <20091118142512.1313744e@perceptron>
Thanks Jan, Jason, Jonathan, and Thomas for your response, your thoughts and
concerns are enlightening....
Jan Kruger wrote...
> git config --global alias.commitx '!git add . && git commit'
> git config --global alias.checkoutx '!git clean && git checkout'
Thank you. Being new to git, I did not know that such aliasing was available
within it.
Jason Sewell wrote...
> If you have a bunch of debugging code sitting around in your working tree
after you've tracked down a
> problem, you don't want to commit all of those printfs, etc. - you want to
commit the fix. This has
> ramifications from making diffs of history cleaner to making git bisect
actually useful.
One of the concerns I have with the manual pick-n-commit is that you can
forget a file or two. Consequently, unless you do a clean checkout and test
of the commit, you don't know that your publishable version even compiles.
It seems safer to commit the entirety of your work in its working state and
then do a clean checkout from a dedicated publishable branch and manually
merge the changes in that, test, and commit.
It seems the intuitive model is to treat version control as applying to the
whole document, not parts of it. In this respect the document is defined by
the IDE, namely the entire solution, warts and all. When you start
selectively saving parts of the document then you are doing two things,
versioning and publishing; and at the same time. This was a critical flaw in
older version control approaches because the software solution document is a
file system sub-tree.
What you termed the debugging/printf's I would treat as a distinctions
between a debug vs. a release version that may be suitably delineated by
#define's or preferably separate unit tests assemblies. If I must prune
prior to committing; however, then it seems reverting spurious printf's may
offer a more reliable and automatable technique than ensuring that I have
added all the new class files, resource files, text files, sub projects,
etc; that may constitute the "fix." Once so selectively reverted I can test
and commit such a publishable version.
Jason Sewell wrote...
> Isn't fastidiously maintaining a .gitignore file to contain everything
you *don't* want in the project more confusing
> than explicitly specifying things you *do* want in the project?
This is git ignore for "cleaning prior to a check" and git ignore for
"adding to index" and is not an either or. You would specify what you don't
want to version tracked as normal but you can also stipulate what you don't
want to be deleted during a clean restore (which should otherwise completely
wipe the folder prior to restoring a specific commit). This would permit
embedding non-version elements within the version tree for whatever reason
you find necessary.
Thomas Rast wrote...
> That would require supernaturally good maintenance of your .gitignore to
avoid adding or (worse) nuking files by accident.
On the contrary, the approach would all but eliminate the possibility of
loss of data since you would not manually (and therefore error prone-ingly)
pruning until after a commit. In fact, one might default automatic commits
(if required) prior to checkouts or at least an alert system when
uncommitted changes exists.
Thanks again for your input.
^ permalink raw reply
* Re: Hey - A Conceptual Simplication....
From: Jakub Narebski @ 2009-11-18 19:40 UTC (permalink / raw)
To: George Dennie; +Cc: 'Jan Krüger', git
In-Reply-To: <008401ca6880$33d7e550$9b87aff0$@com>
"George Dennie" <gdennie@pospeople.com> writes:
> Thanks Jan, Jason, Jonathan, and Thomas for your response, your thoughts and
> concerns are enlightening....
> Jason Sewell wrote...
>
> > If you have a bunch of debugging code sitting around in your working tree
> > after you've tracked down a problem, you don't want to commit all
> > of those printfs, etc. - you want to commit the fix. This has
> > ramifications from making diffs of history cleaner to making git
> > bisect actually useful.
>
> One of the concerns I have with the manual pick-n-commit is that you can
> forget a file or two.
I don't think that this concern is valid.
The files which make project are those defined in Makefile or
equivalent project file, _not_ all files (or even all files of
specific type / extension) that do happen to reside in given
directory. And those files whould be known to git, either added when
importing project into git, or added when they were created. And if
they are known it is enough to use "git commit -a" to pick all
changes.
So I don't see how you can 'forget a file or two'.
Are those *theoretical* concerns, or is it something that happened to
you doring using git?
> Consequently, unless you do a clean checkout and test
> of the commit, you don't know that your publishable version even compiles.
> It seems safer to commit the entirety of your work in its working state and
> then do a clean checkout from a dedicated publishable branch and manually
> merge the changes in that, test, and commit.
That's what
git stash --keep-index
is for.
That, and continuous integration repository, with it's hooks.
>
> It seems the intuitive model is to treat version control as applying to the
> whole document, not parts of it. In this respect the document is defined by
> the IDE, namely the entire solution, warts and all.
Yes, and IDE has project file which defines which files are in
project, just like version control system has it's tracked files.
> When you start
> selectively saving parts of the document then you are doing two things,
> versioning and publishing; and at the same time. This was a critical flaw in
> older version control approaches because the software solution document is a
> file system sub-tree.
Atomic commits are important, but the distinction between tracked
files, (untracked) ignored files, and files in "limbo" state (neither
tracked nor ignored) is orthogonal to having atomic commits.
> Jason Sewell wrote...
>
> > Isn't fastidiously maintaining a .gitignore file to contain
> > everything you *don't* want in the project more confusing than
> > explicitly specifying things you *do* want in the project?
>
> This is git ignore for "cleaning prior to a check" and git ignore for
> "adding to index" and is not an either or. You would specify what you don't
> want to version tracked as normal but you can also stipulate what you don't
> want to be deleted during a clean restore (which should otherwise completely
> wipe the folder prior to restoring a specific commit). This would permit
> embedding non-version elements within the version tree for whatever reason
> you find necessary.
And this is supposedly easier to use? I don't think so.
> Thomas Rast wrote...
>
> > That would require supernaturally good maintenance of your
> > .gitignore to avoid adding or (worse) nuking files by accident.
>
> On the contrary, the approach would all but eliminate the possibility of
> loss of data since you would not manually (and therefore error prone-ingly)
> pruning until after a commit. In fact, one might default automatic commits
> (if required) prior to checkouts or at least an alert system when
> uncommitted changes exists.
What? I cannot understand you here.
I think that automatic pruning of non-versioned files is _more_ error
prone than manual deleting of files. And much more error prone that
just keeping non-ignored and non-tracked files.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Hey - A Conceptual Simplication....
From: Jason Sewall @ 2009-11-18 19:52 UTC (permalink / raw)
To: Jakub Narebski; +Cc: George Dennie, Jan Krüger, git
In-Reply-To: <m37htnd3kb.fsf@localhost.localdomain>
Sorry for the 2x post, George; forgot to include the list in my reply....
On Wed, Nov 18, 2009 at 1:51 PM, George Dennie <gdennie@pospeople.com> wrote:
[some cleanup of quote line wrapping]
> Jason Sewall wrote...
>> If you have a bunch of debugging code sitting around in your
>> working tree after you've tracked down a problem, you don't want to
>> commit all of those printfs, etc. - you want to commit the
>> fix. This has ramifications from making diffs of history cleaner to
>> making git bisect actually useful.
> One of the concerns I have with the manual pick-n-commit is that you
> can forget a file or two. Consequently, unless you do a clean
> checkout and test of the commit, you don't know that your
> publishable version even compiles. It seems safer to commit the
> entirety of your work in its working state and then do a clean
> checkout from a dedicated publishable branch and manually merge the
> changes in that, test, and commit.
I find git status very useful in preparing a commit; untracked (and
'un-ignored') files are listed right there and I can if there are new
source files that are not present but not tracked. You could even add
a 'pre-commit hook' to make sure that you don't have any untracked *.c
(or whatever) files before you actually make the commit.
As to 'publishable' version, it's probably a good idea to run 'make
distcheck' or the equivalent before making a release anyway.
> It seems the intuitive model is to treat version control as applying
> to the whole document, not parts of it. In this respect the document
> is defined by the IDE, namely the entire solution, warts and
> all. When you start selectively saving parts of the document then
> you are doing two things, versioning and publishing; and at the same
> time. This was a critical flaw in older version control approaches
> because the software solution document is a file system sub-tree.
I find this leads to big, shapeless commits and, as I mentioned
before, it seriously limits the utility of 'git bisect'. I also fail
to see how 'selectively saving parts of the document' is versioning
and publishing - what is the publishing part? The act of committing
is one thing (and 'saving parts of the document' is one conceivable
name for it) and publishing another. Your workflow may vary, but
before actually 'publishing' (perhaps pushing out to a public repo, or
merging into a public branch), it's probably a good idea to test the
code with whatever system you use anyway.
> What you termed the debugging/printf's I would treat as a
> distinctions between a debug vs. a release version that may be
> suitably delineated by #define's or preferably separate unit tests
> assemblies. If I must prune prior to committing; however, then it
> seems reverting spurious printf's may offer a more reliable and
> automatable technique than ensuring that I have added all the new
> class files, resource files, text files, sub projects, etc; that may
> constitute the "fix." Once so selectively reverted I can test and
> commit such a publishable version.
What if you are hacking away and make changes to several parts of the
code at once? Making the commits as fine-grained as possible makes it
easier to cherry-pick, bisect, and understand the history.
As to debugging code, I admit I sometimes will use git gui or git add
-p to stage just what I want and then put whatever is 'left over' in a
branch that I might use again later if another bug comes up. Then I
can reset --hard my 'working' branch and the debugging code is gone.
> Jason Sewell wrote...
>> Isn't fastidiously maintaining a .gitignore file to contain
>> everything you *don't* want in the project more confusing than
>> explicitly specifying things you *do* want in the project?
>
> This is git ignore for "cleaning prior to a check" and git ignore
> for "adding to index" and is not an either or. You would specify
> what you don't want to version tracked as normal but you can also
> stipulate what you don't want to be deleted during a clean restore
> (which should otherwise completely wipe the folder prior to
> restoring a specific commit). This would permit embedding
> non-version elements within the version tree for whatever reason you
> find necessary.
Perhaps I don't understand your scheme, but it sounds like you're
advocating 2 .gitignores:
* .gitignore_track; with everything you don't automatically staged but
which can be trashed by your cleaning checkout
* .gitignore_keep; with things you don't want staged but which
shouldn't be deleted by git during cleaning
That seems even more confusing. I'm actually having trouble seeing
why you want this untracked-file nuking checkout at all. Care to give
an example?
> Thomas Rast wrote...
>> That would require supernaturally good maintenance of your
>> .gitignore to
> avoid adding or (worse) nuking files by accident.
>
> On the contrary, the approach would all but eliminate the
> possibility of loss of data since you would not manually (and
> therefore error prone-ingly) pruning until after a commit. In fact,
> one might default automatic commits (if required) prior to checkouts
> or at least an alert system when uncommitted changes exists.
Who is pruning after a commit? Once nice thing about checkout is that
it will refuse to move to a different commit if there are files that
will get trashed. Then you can say 'oops, I should stash/commit/nuke
that stuff before I change HEAD.
Jason
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Philip Hofstetter @ 2009-11-18 19:57 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091118172424.GA24416@coredump.intra.peff.net>
Hello,
On Wed, Nov 18, 2009 at 6:24 PM, Jeff King <peff@peff.net> wrote:
> 1. It creates a bad user experience. You are not unreasonable for
> wanting to put some specific text in your commit message. Having
> git come back and say "oops, I might get confused by this later"
> just seems like an annoyance to the user.
agreed, though it's not that bad: when learning git, you will be
confronted with the fact that the commit message has a few things that
are special (well. it's doesn't break git, but the first line should
be < 56 chars in length for example).
Not being able to have From: lines in them that are not describing an
author would then just be one of them.
> 2. Mailinfo has to deal with data created by older versions of git. So
> in your case, the rebase was a bomb waiting to go off. If we can
> fix it so that an existing bomb doesn't go off, rather than not
> creating the bomb in the first place, then we are better off.
This is a very good point. I didn't quite think about that.
> 4. Commit messages can come from other places than "git commit". What
> should we do with a commit message like this that is imported from
> SVN? Reject the import? Munge the message?
I would leave that to the tool that does the import. Probably it would
have to munge it. Yes.
I DO see though that implementing the check at commit time would lead
to problems popping up at other places.
> Of course all of that presupposes that we can correctly handle the
> existing data after the fact. Even with my patch, you still can't write
> "From: foo@example.com" as the first line of your commit body. But that
can't you? IMHO it would just attribute the commit to foo@example.com
which can be an equally bad, if not worse thing (I'm saying that
without the needed knowledge about git internals to really be sure, so
take this with a grain of salt)
I just have a bad feeling about trying out heuristics to see whether
thing thing after from: is an email address or not as email addresses
are notoriously hard to detect.
Typing a commit message and applying a patch from an email should be
separate things and should be handled separately. Currently they are
not and this is what's causing the problem in the first place.
Maybe that --strict thing is actually a good thing in the long run,
even though I don't quite like it either :-)
Interesting problem to have though.
Philip
^ permalink raw reply
* Re: Hey - A Conceptual Simplication....
From: Linus Torvalds @ 2009-11-18 20:36 UTC (permalink / raw)
To: George Dennie; +Cc: git
In-Reply-To: <005a01ca684e$71a1d710$54e58530$@com>
On Wed, 18 Nov 2009, George Dennie wrote:
>
> The Git model does not seem to go far enough conceptually, for some
> unexplainable reason...
Others already mentioned this, but the concept you missed is the git
'index', which is actually very central (it is actually the first part of
git written, before even the object database) but is something that most
people who get started with git can (and do) ignore.
Now, admittedly, for casual use it's not always clear _why_ the index is
so central, so the fact that you overlooked it is certainly easy to
understand. Just take my word for it: to truly understand git, you do need
to understand the index.
You can ignore it for a long time, because one of the primary reasons for
it existing is about performance. That happens to be a primary goal of
git, of course, but some people always think it's "just performance". It's
way more fundamental than that.
So the way you can start getting used to the index is to think of it as a
way to avoid having to do a full 'readdir()' on the whole tree to figure
out what is in there, and avoiding having to read all the files to check
that their contents still match.
Of course, if that was _all_ the index did, it could be seen purely as a
cache, and have no semantic visibility at all. And that's not the case:
the index does have real semantic visibility.
The first time you'll see it is when you decide to stage your changes in
parts. The index is what allows you to _not_ always commit all your
changes exactly because git keeps track of something more than _just_ your
whole current working tree.
A special case (but a really useful one) of the "staging your changes in
parts" is when you do merges. Now, most people don't do merges like I do
(what, average of 5 merges per day, day in and day out), so most people
don't care quite as deeply as I do, but if you ever do a merge where 99%
merged cleanly, and 1% did not (which is the common case for conflicts),
you'll really understand why having a system that keeps track of the parts
that merged cleanly is _critical_.
So for merges, the index keeps track of what merged cleanly, and what
didn't, and what the original state for the not-clean stuff was. And as
somebody who probably does more merges than likely any other human in the
history of the world, I can state with some authority that any source
control model that doesn't have this is fundamentally broken.
So the index is really _really_ important. Even if you can ignore it most
of the time. And the index is why you don't have a model of "always just
track the exact tree state".
Linus
^ permalink raw reply
* Re: th/remote-usage
From: Tim Henigan @ 2009-11-18 21:28 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20091118114808.GA13346@progeny.tock>
Hi and thanks for your review.
On Wed, Nov 18, 2009 at 6:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>> [New Topics]
>>
>> * th/remote-usage (2009-11-16) 1 commit.
>> - git remote: Separate usage strings for subcommands
>
> Glancing at pu^2, I had two small nitpicks: [<options>...] is five
> characters longer than strictly necessary
I based my patch on what I found in other builtin functions (such
as push and diff). That being said, I don't think that either my
original patch or your updated version is completely correct.
The choices seem to be:
(1) [<options>...]: My original based on my interpretation of
IEEE 1003.1. [1]
(2) [options]: Your proposal, which drops both the '<>' and '...'.
(3) <options>: Used in builtin-diff.c. Which does not show
that the options are -- optional.
(4) [<options>]: What I now believe is correct (based on the
current implementation of builtin-push.c). This drops the
'...' which IEEE 1003.1 defines as allowing multiple options
to be specified, but it conforms to the conventions in other
commands.
There does not (yet) seem to be consistency in how options
are presented. My current plan is to change the patch to
use choice #4, but if Junio has a chance to comment, I will
of course defer to his decision.
I will send an updated patch that implements choice #4 as
soon as I can (should be within the next 12 hours).
> and the argument to git
> remote set-head is not actually optional.
This was obviously an oversight on my part. I will include the
fix in the next version.
...and from your second email:
> Another option would be to make the strings into static
> variables.
Thanks for the analysis, but I don't plan to include this change
unless specifically requested.
[1]: http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html
^ permalink raw reply
* Re: [PATCH] Give the hunk comment its own color
From: Junio C Hamano @ 2009-11-18 21:56 UTC (permalink / raw)
To: Jeff King; +Cc: Bert Wesarg, git
In-Reply-To: <20091118142320.GA1220@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> PS I almost complained about your default of "magenta" as the same as
> the meta color before I remembered that magenta meta is a personal
> setting I use.
On black-on-white terminals, cyan tends to be less visible, and I think
that is the whole point of painting the hunk header @@ .. @@ in that
color--- make it less distracting).
But the function name on the line is not something that should be made
less visible---if that part of the line were a meaningless cruft, we
wouldn't have configurable funcname patterns after all.
I would suggest "normal" as the neutral default. After all, the purpose
of the funcname in the hunk header is to give context to people who read
patches.
> I'm not sure what is the best way to arrive at a default color for
> something like this. Arguing about it really is almost the definition of
> bikeshedding. Maybe next year's git survey should contain a special
> section on colors, and majority should rule. :)
Sorry, but this is no democracy ;-)
^ permalink raw reply
* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Junio C Hamano @ 2009-11-18 22:05 UTC (permalink / raw)
To: Bert Wesarg; +Cc: git
In-Reply-To: <1258560919-28054-1-git-send-email-bert.wesarg@googlemail.com>
Bert Wesarg <bert.wesarg@googlemail.com> writes:
> I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this
> the test t4252-am-options.sh fails because it calls grep with a .rej file:
Yuck. Will apply.
That actually makes me worried about a different issue.
Do we kill that environment variable when we call out to external grep in
grep.c? If not, we should. An alternative is to teach our internal one
to also honor it, but I personally do not find it too attractive to mimic
the design mistake of GREP_OPTIONS myself.
^ permalink raw reply
* Re: [PATCH 2/2] ls-tree: migrate to parse-options
From: Junio C Hamano @ 2009-11-18 22:19 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git
In-Reply-To: <4B024A89.2010108@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Junio C Hamano wrote:
>>
>> @@ -24,7 +24,7 @@ static int chomp_prefix;
>> static const char *ls_tree_prefix;
>> static const char * const ls_tree_usage[] = {
>> - "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]",
>> + "git ls-tree <options> <tree-ish> [path...]",
>> NULL
>> };
>>
>
> Is it [<options>] or [<options>...] or <options> or even
> <options>... though?
Output from "git grep -e '<option' -- '*.c'" indicates that it
should be "[<options>]"; thanks for spotting.
^ permalink raw reply
* Re: [PATCH] Give the hunk comment its own color
From: Jeff King @ 2009-11-18 22:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Bert Wesarg, git
In-Reply-To: <7vaayjebu5.fsf@alter.siamese.dyndns.org>
On Wed, Nov 18, 2009 at 01:56:34PM -0800, Junio C Hamano wrote:
> On black-on-white terminals, cyan tends to be less visible, and I think
> that is the whole point of painting the hunk header @@ .. @@ in that
> color--- make it less distracting).
Hmm. I find cyan-on-white gratingly ugly instead of "less distracting",
but then again, I find black-on-white terminals to be eye-searing in
general.
> I would suggest "normal" as the neutral default. After all, the purpose
> of the funcname in the hunk header is to give context to people who read
> patches.
I think that is sensible.
-Peff
^ permalink raw reply
* [PATCH] git am/mailinfo: Don't look at in-body headers when rebasing
From: Lukas Sandström @ 2009-11-18 22:45 UTC (permalink / raw)
To: Jeff King; +Cc: Lukas Sandström, Philip Hofstetter, git
In-Reply-To: <20091118164208.GB15184@coredump.intra.peff.net>
When we are rebasing we know that the header lines in the
patch are good and that we don't need to pick up any headers
from the body of the patch.
This makes it possible to rebase commits whose commit message
start with "From" or "Date".
Test vectors by Jeff King.
Signed-off-by: Lukas Sandström <luksan@gmail.com>
---
Jeff King wrote:
>> Some solutions I can think of are:
>>
>> 1. Improve the header-finding heuristic to actually look for something
>> more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
>> head which other headers we handle in this position. Probably
>> Date, too).
>>
>> 2. Give mailinfo a "--strict" mode to indicate that it is directly
>> parsing the output of format-patch, and not some random email. Use
>> --strict when invoking "git am" via "git rebase".
>
> Solution (2) seemed like a lot of work, so here is the relatively small
> solution (1). I think looking for <.*@.*> is too restrictive, as people
> may be using:
>
This is an implementation of solution (2). Not much work, but I might
have missed something. git-mailinfo usally breaks when I touch it,
but the testsuite passes with this patch, including the extra test
vectors from Jeff.
The actual change is that mailinfo doesn't look for in-body headers
at all if --no-inbody-headers is passed. git-am now passes this option
to mailinfo when rebasing.
This won't handle the case when a "bad" patch is passed to git-am from
somewhere else than git rebase.
/Lukas
builtin-mailinfo.c | 5 +++++
git-am.sh | 13 ++++++++++---
t/t5100-mailinfo.sh | 6 +++++-
t/t5100/info0015 | 5 +++++
t/t5100/info0015--no-inbody-headers | 5 +++++
t/t5100/info0016 | 5 +++++
t/t5100/info0016--no-inbody-headers | 5 +++++
t/t5100/msg0015 | 2 ++
t/t5100/msg0015--no-inbody-headers | 3 +++
t/t5100/msg0016 | 2 ++
t/t5100/msg0016--no-inbody-headers | 4 ++++
t/t5100/patch0015 | 8 ++++++++
t/t5100/patch0015--no-inbody-headers | 8 ++++++++
t/t5100/patch0016 | 8 ++++++++
t/t5100/patch0016--no-inbody-headers | 8 ++++++++
t/t5100/sample.mbox | 33
+++++++++++++++++++++++++++++++++
16 files changed, 116 insertions(+), 4 deletions(-)
create mode 100644 t/t5100/info0015
create mode 100644 t/t5100/info0015--no-inbody-headers
create mode 100644 t/t5100/info0016
create mode 100644 t/t5100/info0016--no-inbody-headers
create mode 100644 t/t5100/msg0015
create mode 100644 t/t5100/msg0015--no-inbody-headers
create mode 100644 t/t5100/msg0016
create mode 100644 t/t5100/msg0016--no-inbody-headers
create mode 100644 t/t5100/patch0015
create mode 100644 t/t5100/patch0015--no-inbody-headers
create mode 100644 t/t5100/patch0016
create mode 100644 t/t5100/patch0016--no-inbody-headers
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index c90cd31..a81526e 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -26,6 +26,7 @@ static struct strbuf charset = STRBUF_INIT;
static int patch_lines;
static struct strbuf **p_hdr_data, **s_hdr_data;
static int use_scissors;
+static int use_inbody_headers = 1;
#define MAX_HDR_PARSED 10
#define MAX_BOUNDARIES 5
@@ -771,6 +772,8 @@ static int handle_commit_msg(struct strbuf *line)
return 0;
if (still_looking) {
+ if (!use_inbody_headers)
+ still_looking = 0;
strbuf_ltrim(line);
if (!line->len)
return 0;
@@ -1033,6 +1036,8 @@ int cmd_mailinfo(int argc, const char **argv,
const char *prefix)
use_scissors = 1;
else if (!strcmp(argv[1], "--no-scissors"))
use_scissors = 0;
+ else if (!strcmp(argv[1], "--no-inbody-headers"))
+ use_inbody_headers = 0;
else
usage(mailinfo_usage);
argc--; argv++;
diff --git a/git-am.sh b/git-am.sh
index c132f50..96869a2 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -289,7 +289,7 @@ split_patches () {
prec=4
dotest="$GIT_DIR/rebase-apply"
sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors=
+resolvemsg= resume= scissors= no_inbody_headers=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -322,7 +322,7 @@ do
--abort)
abort=t ;;
--rebasing)
- rebasing=t threeway=t keep=t scissors=f ;;
+ rebasing=t threeway=t keep=t scissors=f no_inbody_headers=t ;;
-d|--dotest)
die "-d option is no longer supported. Do not use."
;;
@@ -448,6 +448,7 @@ else
echo "$utf8" >"$dotest/utf8"
echo "$keep" >"$dotest/keep"
echo "$scissors" >"$dotest/scissors"
+ echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
echo "$GIT_QUIET" >"$dotest/quiet"
echo 1 >"$dotest/next"
if test -n "$rebasing"
@@ -495,6 +496,12 @@ t)
f)
scissors=--no-scissors ;;
esac
+if test "$(cat "$dotest/no_inbody_headers")" = t
+then
+ no_inbody_headers=--no-inbody-headers
+else
+ no_inbody_headers=
+fi
if test "$(cat "$dotest/quiet")" = t
then
GIT_QUIET=t
@@ -549,7 +556,7 @@ do
# by the user, or the user can tell us to do so by --resolved flag.
case "$resume" in
'')
- git mailinfo $keep $scissors $utf8 "$dotest/msg" "$dotest/patch" \
+ git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
<"$dotest/$msgnum" >"$dotest/info" ||
stop_here $this
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 0279d07..50e13c1 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
- test `cat last` = 14'
+ test `cat last` = 16'
check_mailinfo () {
mail=$1 opt=$2
@@ -30,6 +30,10 @@ do
if test -f "$TEST_DIRECTORY"/t5100/msg$mail--scissors
then
check_mailinfo $mail --scissors
+ fi &&
+ if test -f "$TEST_DIRECTORY"/t5100/msg$mail--use-first-header
+ then
+ check_mailinfo $mail --no-inbody-headers
fi
'
done
diff --git a/t/t5100/info0015 b/t/t5100/info0015
new file mode 100644
index 0000000..0114f10
--- /dev/null
+++ b/t/t5100/info0015
@@ -0,0 +1,5 @@
+Author:
+Email:
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0015--no-inbody-headers
b/t/t5100/info0015--no-inbody-headers
new file mode 100644
index 0000000..c4d8d77
--- /dev/null
+++ b/t/t5100/info0015--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0016 b/t/t5100/info0016
new file mode 100644
index 0000000..38ccd0d
--- /dev/null
+++ b/t/t5100/info0016
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: bogus
+
diff --git a/t/t5100/info0016--no-inbody-headers
b/t/t5100/info0016--no-inbody-headers
new file mode 100644
index 0000000..f4857d4
--- /dev/null
+++ b/t/t5100/info0016--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
new file mode 100644
index 0000000..9577238
--- /dev/null
+++ b/t/t5100/msg0015
@@ -0,0 +1,2 @@
+- a list
+ - of stuff
diff --git a/t/t5100/msg0015--no-inbody-headers
b/t/t5100/msg0015--no-inbody-headers
new file mode 100644
index 0000000..be5115b
--- /dev/null
+++ b/t/t5100/msg0015--no-inbody-headers
@@ -0,0 +1,3 @@
+From: bogosity
+ - a list
+ - of stuff
diff --git a/t/t5100/msg0016 b/t/t5100/msg0016
new file mode 100644
index 0000000..0d9adad
--- /dev/null
+++ b/t/t5100/msg0016
@@ -0,0 +1,2 @@
+and some content
+
diff --git a/t/t5100/msg0016--no-inbody-headers
b/t/t5100/msg0016--no-inbody-headers
new file mode 100644
index 0000000..1063f51
--- /dev/null
+++ b/t/t5100/msg0016--no-inbody-headers
@@ -0,0 +1,4 @@
+Date: bogus
+
+and some content
+
diff --git a/t/t5100/patch0015 b/t/t5100/patch0015
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0015--no-inbody-headers
b/t/t5100/patch0015--no-inbody-headers
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015--no-inbody-headers
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016 b/t/t5100/patch0016
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016--no-inbody-headers
b/t/t5100/patch0016--no-inbody-headers
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016--no-inbody-headers
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 13fa4ae..de10312 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -650,3 +650,36 @@ index b0b5d8f..461c47e 100644
convert_to_utf8(line, charset.buf);
--
1.6.4.1
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: bogosity
+ - a list
+ - of stuff
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+Date: bogus
+
+and some content
+
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
--
1.6.4.4
^ permalink raw reply related
* Re: [PATCH 2/2] ls-tree: migrate to parse-options
From: Thiago Farina @ 2009-11-18 23:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7veinvcw7w.fsf@alter.siamese.dyndns.org>
On Wed, Nov 18, 2009 at 8:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>>
>>> @@ -24,7 +24,7 @@ static int chomp_prefix;
>>> static const char *ls_tree_prefix;
>>> static const char * const ls_tree_usage[] = {
>>> - "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]",
>>> + "git ls-tree <options> <tree-ish> [path...]",
>>> NULL
>>> };
>>>
>>
>> Is it [<options>] or [<options>...] or <options> or even
>> <options>... though?
>
> Output from "git grep -e '<option' -- '*.c'" indicates that it
> should be "[<options>]"; thanks for spotting.
$ git grep -e '<option' -- '*.c' | wc -l
4
$ git grep -e '\[options' -- '*.c' | wc -l
42
$ git grep -e '\[<options' -- '*.c' | wc -l
2
Shouldn't be just "[options]"?
^ 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