* Re: [JGIT] Request for help
From: Jonas Fonseca @ 2009-09-04 12:41 UTC (permalink / raw)
To: Mark Struberg; +Cc: git, Gabe McArthur
In-Reply-To: <554991.93608.qm@web27807.mail.ukl.yahoo.com>
On Fri, Sep 4, 2009 at 03:33, Mark Struberg<struberg@yahoo.de> wrote:
>> From: Gabe McArthur <gabriel.mcarthur@gmail.com>
> >
>> I'll try to submit a full patch later, using your
>> conventions.
I have a question as well:
Support for using find bug is part of the Eclipse configuration (see
org.spearce.jgit/findBugs/), and I know that there's a find bug plugin
for Maven. From looking at sonatype's JGit repositories it is not
integrated. Have you managed to include it?
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Johannes Schindelin @ 2009-09-04 12:35 UTC (permalink / raw)
To: Gerrit Pape; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <20090904110936.6663.qmail@046e1bfbf7e41d.315fe32.mid.smarden.org>
Hi,
On Fri, 4 Sep 2009, Gerrit Pape wrote:
> On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > -- snipsnap --
> > [PATCH] git help -a: do not look for a repository
>
> Perfect, thanks.
>
> Acked-by: Gerrit Pape <pape@smarden.org>
Hmm... I haven't checked if 'git help -a' wants to discover the (possibly
repository-specific) aliases. Have you?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Johannes Schindelin @ 2009-09-04 12:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7vk50fugpn.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, 4 Sep 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > The special case is "http://" and "https://" which might indicate foreign
> > VCS repositories.
> >
> > In all other cases, I am afraid that you are complicating the glove.
> >
> > Remember: the whole purpose of the "foreign VCS" helpers is user
> > convenience.
>
> Sorry, but you completely lost me here.
My point was that the ambiguity _only_ applies to http:// and https://
URLs, as you illustrated yourself:
> Here are two URLs that follows your "user convenience" principle.
>
> http://example.xz/repos/frotz.git/
> http://example.xz/repo/frotz/trunk/
There is no ambiguity about hg://, svn://, etc. None.
Some "URLs" do not look like "URLs" at all, e.g. :ext:user@host:/module
for CVS repositories. I haven't really thought about a convenient way to
specify these, but I could imagine that indeed something like
"cvs::ext:usr@host:/module" would make sense, and still be an intuitive
interface that does not break down with "git clone".
Likewise, I imagine that "svn::http://example.xz/repo/frotz/trunk" (or
even "svn::std::http://example.xz/repo/frotz") are not _too_ unintuitive.
But my real point still stands: "git clone hg://example.xz/repos/blub"
should Just Work.
Oh, and I definitely do not want to expose an _implementation detail_ such
as "we use cURL" in the name of the remote helper. That would just be bad
style in my book.
> How do you tell, without relying on .git and trunk, the former is a git
> repository and wants the dumb walker transport to fetch from, while the
> latter is probably a svn and you would either use "svn checkout", or use
> "git clone" on it via the svn helper?
>
> Well, you don't.
>
> One possible "convenient user interface" would be to say
>
> svn+http://example.xz/repo/frotz/trunk/
>
> (or use :: instead of + as the helper-name separator, as we agreed not to
> decide on it)
Now that you mention it, the main issue was the ambiguity that
svn:/path/to/repo
should actually be an ssh "URL". But I think that the simple fact that a
"://" in the URL (and if that is not sufficient, something like a
"<vcs>::" prefix) make non-ssh URLs distinct enough to decide robustly
what type the URL is.
> This would give us
>
> (1) it is clear that it literally is what you would give to git and
> trigger the svn helper; and
>
> (2) to people who know how canonical git URLs with foreign helper are
> spelled, it also is clear that you can use "svn checkout" on
> everything after "svn+" in it.
The only problem is that you cannot use "git-remote-svn+http" as helper,
as "+" are not valid filename characters on Windows. However, you could
have a "git-remote-svn" handling both "svn://" and "svn+" prefixes.
> Compared to that, if you do not have any such prefix, how would that be
> more convenient to the users?
Indeed, I made myself misunderstood. I think that for _a lot_ of
repository URLs, there are naturally distinctive-enough prefixes. IMHO we
should make use of that, for a substantially improved user experience (as
opposed to, say, the user experience for unfortunate CVS users who would
like to establish a git-svn-like workflow).
Summary: I think that for most URLs, "<protocol>://" is enough to tell
which helper to call ("http://" means Git, tough).
For those URLs, where this is not sufficient, a "<vcs>+" should be good
enough, or if you really want, "<vcs>::". As the helper gets the complete
URL, it can figure out how to proceed from here, without any need for
core Git to know how.
Ciao,
Dscho
^ permalink raw reply
* Re: [JGIT] Request for help
From: Mark Struberg @ 2009-09-04 12:27 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: git, Gabe McArthur
In-Reply-To: <2c6b72b30909040522l4f27ed36u10f47517d42d57a3@mail.gmail.com>
Hi Jonas!
See the following documentation:
http://maven.apache.org/plugins/maven-release-plugin/index.html
http://maven.apache.org/plugins/maven-release-plugin/examples/prepare-release.html
http://maven.apache.org/plugins/maven-release-plugin/examples/perform-release.html
There is also a freely available book from sonatype [1] which contains all the documemtation you need - plus fn lot more :)
LieGrue,
strub
[1] http://www.sonatype.com/book/
--- On Fri, 9/4/09, Jonas Fonseca <jonas.fonseca@gmail.com> wrote:
> From: Jonas Fonseca <jonas.fonseca@gmail.com>
> Subject: Re: [JGIT] Request for help
> To: "Mark Struberg" <struberg@yahoo.de>
> Cc: git@vger.kernel.org, "Gabe McArthur" <gabriel.mcarthur@gmail.com>
> Date: Friday, September 4, 2009, 2:22 PM
> On Fri, Sep 4, 2009 at 03:33, Mark
> Struberg<struberg@yahoo.de>
> wrote:
> > .) we don't need a tag.sh any more if we work with
> maven. Maven now has the maven-scm-provider-gitexe activated
> by default (since early 2008), so
> > mvn release:prepare
> > mvn release:perform
> > should work if we set the proper <scm> section.
> Any feedback or bugreporting on the maven-git integration is
> highly welcome btw ;)
>
> If tag_jgit.sh goes away, it could be nice to add a
> document showing
> how releasing/tagging it's done the maven way.
>
> --
> Jonas Fonseca
>
^ permalink raw reply
* Re: [JGIT] Request for help
From: Jonas Fonseca @ 2009-09-04 12:22 UTC (permalink / raw)
To: Mark Struberg; +Cc: git, Gabe McArthur
In-Reply-To: <554991.93608.qm@web27807.mail.ukl.yahoo.com>
On Fri, Sep 4, 2009 at 03:33, Mark Struberg<struberg@yahoo.de> wrote:
> .) we don't need a tag.sh any more if we work with maven. Maven now has the maven-scm-provider-gitexe activated by default (since early 2008), so
> mvn release:prepare
> mvn release:perform
> should work if we set the proper <scm> section. Any feedback or bugreporting on the maven-git integration is highly welcome btw ;)
If tag_jgit.sh goes away, it could be nice to add a document showing
how releasing/tagging it's done the maven way.
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly
From: Johannes Sixt @ 2009-09-04 12:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1252061718-11579-3-git-send-email-gitster@pobox.com>
Junio C Hamano schrieb:
> The command tries to strip blank lines at the end of the file added by a
> patch. However, if the original ends with blank lines, often the patch
> hunk ends like this:
>
> @@ -l,5 +m,7 @@$
> _context$
> _context$
> -deleted$
> +$
> +$
> +$
> _$
> _$
>
> where _ stands for SP and $ shows a end-of-line. This example patch adds
> three trailing blank lines, but the code fails to notice it, because it
> only pays attention to added blank lines at the very end of the hunk. In
> this example, the three added blank lines do not appear textually at the
> end in the patch, even though you can see that they are indeed added at
> the end, if you rearrange the diff like this:
>
> @@ -l,5 +m,7 @@$
> _context$
> _context$
> -deleted$
> _$
> _$
> +$
> +$
> +$
>
> Fix this by not resetting the number of (candidate) added blank lines at
> the end when the loop sees a context line that is empty.
After reading this explanation, I was worried that added blank lines that
are at the end of a patch but apply in the middle of a file would be
mis-attributed as blank lines at EOF. But appearently, they are not, i.e.
such added blank lines are not removed. Could you squash in this test case
that checks for this condition.
-- Hannes
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index ba2b7f9..fedc8b9 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -189,4 +189,16 @@ test_expect_success 'blank at EOF with --whitespace=fix (3)' '
test_cmp expect one
'
+test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' '
+ { echo a; echo b; echo; echo; echo; echo; echo; echo d; } >one &&
+ git add one &&
+ { echo a; echo c; echo; echo; echo; echo; echo; echo; echo d; } >expect &&
+ cp expect one &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ git apply --whitespace=fix patch &&
+ test_cmp expect one
+'
+
test_done
^ permalink raw reply related
* [PATCH] Do not scramble password read from .cvspass
From: Pascal Obry @ 2009-09-04 11:58 UTC (permalink / raw)
To: git list
Passwords stored in .cvspass are already scrambled, we do not
want to scramble them twice. Only passwords read from the
command line are scrambled.
This fixes a regression introduced by:
b2139dbd72d16e40eddfd5b9ad1314703b39fe65
Signed-off-by: Pascal Obry <pascal@obry.net>
---
git-cvsimport.perl | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 593832d..c5cdcae 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -238,7 +238,10 @@ sub conn {
}
my $rr = ":pserver:$user\@$serv:$port$repo";
- unless ($pass) {
+ if ($pass) {
+ $pass = $self->_scramble($pass);
+ } else
+ {
open(H,$ENV{'HOME'}."/.cvspass") and do {
#
:pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah<Z
while (<H>) {
@@ -253,8 +256,6 @@ sub conn {
};
}
- $pass = $self->_scramble($pass);
-
my ($s, $rep);
if ($proxyhost) {
--
1.6.4.2.253.g0b1fac
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net - http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B
^ permalink raw reply related
* Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Gerrit Pape @ 2009-09-04 11:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0909041217370.4605@intel-tinevez-2-302>
On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> -- snipsnap --
> [PATCH] git help -a: do not look for a repository
Perfect, thanks.
Acked-by: Gerrit Pape <pape@smarden.org>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
> ---
>
> builtin-help.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-help.c b/builtin-help.c
> index e1eba77..719aa23 100644
> --- a/builtin-help.c
> +++ b/builtin-help.c
> @@ -416,9 +416,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
> const char *alias;
> load_command_list("git-", &main_cmds, &other_cmds);
>
> - setup_git_directory_gently(&nongit);
> - git_config(git_help_config, NULL);
> -
> argc = parse_options(argc, argv, prefix, builtin_help_options,
> builtin_help_usage, 0);
>
> @@ -429,6 +426,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
> return 0;
> }
>
> + setup_git_directory_gently(&nongit);
> + git_config(git_help_config, NULL);
> +
> if (!argv[0]) {
> printf("usage: %s\n\n", git_usage_string);
> list_common_cmds_help();
> --
> 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
^ permalink raw reply
* [PATCH 8/9] diff --whitespace=warn/error: fix blank-at-eof check
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
The "diff --check" logic used to share the same issue as the one fixed for
"git apply" earlier in this series, in that a patch that adds new blank
lines at end could appear as
@@ -l,5 +m,7 @@$
_context$
_context$
-deleted$
+$
+$
+$
_$
_$
where _ stands for SP and $ shows a end-of-line. Instead of looking at
each line in the patch in the callback, simply count the blank lines from
the end in two versions, and notice the presence of new ones.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 64 +++++++++++++++++++++++++++++++++-----------
t/t4015-diff-whitespace.sh | 7 +++++
2 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/diff.c b/diff.c
index a693d18..c19c476 100644
--- a/diff.c
+++ b/diff.c
@@ -1149,7 +1149,6 @@ struct checkdiff_t {
struct diff_options *o;
unsigned ws_rule;
unsigned status;
- int trailing_blanks_start;
};
static int is_conflict_marker(const char *line, unsigned long len)
@@ -1193,10 +1192,6 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
if (line[0] == '+') {
unsigned bad;
data->lineno++;
- if (!ws_blank_line(line + 1, len - 1, data->ws_rule))
- data->trailing_blanks_start = 0;
- else if (!data->trailing_blanks_start)
- data->trailing_blanks_start = data->lineno;
if (is_conflict_marker(line + 1, len - 1)) {
data->status |= 1;
fprintf(data->o->file,
@@ -1216,14 +1211,12 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
data->o->file, set, reset, ws);
} else if (line[0] == ' ') {
data->lineno++;
- data->trailing_blanks_start = 0;
} else if (line[0] == '@') {
char *plus = strchr(line, '+');
if (plus)
data->lineno = strtol(plus, NULL, 10) - 1;
else
die("invalid diff");
- data->trailing_blanks_start = 0;
}
}
@@ -1437,6 +1430,44 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi
return NULL;
}
+static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
+{
+ char *ptr = mf->ptr;
+ long size = mf->size;
+ int cnt = 0;
+
+ if (!size)
+ return cnt;
+ ptr += size - 1; /* pointing at the very end */
+ if (*ptr != '\n')
+ ; /* incomplete line */
+ else
+ ptr--; /* skip the last LF */
+ while (mf->ptr < ptr) {
+ char *prev_eol;
+ for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
+ if (*prev_eol == '\n')
+ break;
+ if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
+ break;
+ cnt++;
+ ptr = prev_eol - 1;
+ }
+ return cnt;
+}
+
+static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)
+{
+ int l1, l2, at;
+ l1 = count_trailing_blank(mf1, ws_rule);
+ l2 = count_trailing_blank(mf2, ws_rule);
+ if (l2 <= l1)
+ return 0;
+ /* starting where? */
+ at = count_lines(mf1->ptr, mf1->size);
+ return (at - l1) + 1; /* the line number counts from 1 */
+}
+
static void builtin_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -1650,15 +1681,16 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
ecb.priv = &data;
xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
- if ((data.ws_rule & WS_BLANK_AT_EOF) &&
- data.trailing_blanks_start) {
- static char *err;
-
- if (!err)
- err = whitespace_error_string(WS_BLANK_AT_EOF);
- fprintf(o->file, "%s:%d: %s\n",
- data.filename, data.trailing_blanks_start, err);
- data.status = 1; /* report errors */
+ if (data.ws_rule & WS_BLANK_AT_EOF) {
+ int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);
+ if (blank_at_eof) {
+ static char *err;
+ if (!err)
+ err = whitespace_error_string(WS_BLANK_AT_EOF);
+ fprintf(o->file, "%s:%d: %s.\n",
+ data.filename, blank_at_eof, err);
+ data.status = 1; /* report errors */
+ }
}
}
free_and_return:
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index a5d4461..e0b481d 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -341,6 +341,13 @@ test_expect_success 'checkdiff detects new trailing blank lines (1)' '
git diff --check | grep "new blank line"
'
+test_expect_success 'checkdiff detects new trailing blank lines (2)' '
+ { echo a; echo b; echo; echo; } >x &&
+ git add x &&
+ { echo a; echo; echo; echo; echo; } >x &&
+ git diff --check | grep "new blank line"
+'
+
test_expect_success 'checkdiff allows new blank lines' '
git checkout x &&
mv x y &&
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 9/9] diff --color: color blank-at-eof
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
Since the coloring logic processed the patch output one line at a time, we
couldn't easily color code the new blank lines at the end of file.
Reuse the adds_blank_at_eof() function to find where the runs of such
blank lines start, keep track of the line number in the preimage while
processing the patch output one line at a time, and paint the new blank
lines that appear after that line to implement this.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 37 +++++++++++++++++++++++++++----------
t/t4019-diff-wserror.sh | 9 +++++++++
2 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/diff.c b/diff.c
index c19c476..2b285b8 100644
--- a/diff.c
+++ b/diff.c
@@ -491,6 +491,8 @@ struct emit_callback {
struct xdiff_emit_state xm;
int color_diff;
unsigned ws_rule;
+ int blank_at_eof;
+ int lno_in_preimage;
sane_truncate_fn truncate;
const char **label_path;
struct diff_words_data *diff_words;
@@ -547,6 +549,12 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
if (!*ws)
emit_line(ecbdata->file, set, reset, line, len);
+ else if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
+ ecbdata->blank_at_eof &&
+ (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) &&
+ ws_blank_line(line + 1, len - 1, ecbdata->ws_rule))
+ /* Blank line at EOF */
+ emit_line(ecbdata->file, ws, reset, line, len);
else {
/* Emit just the prefix, then the rest. */
emit_line(ecbdata->file, set, reset, line, 1);
@@ -573,9 +581,16 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u
return allot - l;
}
+static int find_preimage_lno(const char *line)
+{
+ char *p = strchr(line, '-');
+ if (!p)
+ return 0; /* should not happen */
+ return strtol(p+1, NULL, 10);
+}
+
static void fn_out_consume(void *priv, char *line, unsigned long len)
{
- int color;
struct emit_callback *ecbdata = priv;
const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
@@ -598,6 +613,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
if (line[0] == '@') {
len = sane_truncate_line(ecbdata, line, len);
+ ecbdata->lno_in_preimage = find_preimage_lno(line);
emit_line(ecbdata->file,
diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
reset, line, len);
@@ -611,7 +627,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
return;
}
- color = DIFF_PLAIN;
if (ecbdata->diff_words) {
if (line[0] == '-') {
diff_words_append(line, len,
@@ -630,14 +645,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
emit_line(ecbdata->file, plain, reset, line, len);
return;
}
- if (line[0] == '-')
- color = DIFF_FILE_OLD;
- else if (line[0] == '+')
- color = DIFF_FILE_NEW;
- if (color != DIFF_FILE_NEW) {
- emit_line(ecbdata->file,
- diff_get_color(ecbdata->color_diff, color),
- reset, line, len);
+
+ if (line[0] != '+') {
+ const char *color =
+ diff_get_color(ecbdata->color_diff,
+ line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
+ ecbdata->lno_in_preimage++;
+ emit_line(ecbdata->file, color, reset, line, len);
return;
}
emit_add_line(reset, ecbdata, line, len);
@@ -1557,6 +1571,9 @@ static void builtin_diff(const char *name_a,
ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
ecbdata.found_changesp = &o->found_changes;
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+ if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
+ ecbdata.blank_at_eof =
+ adds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule);
ecbdata.file = o->file;
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
xecfg.ctxlen = o->context;
diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh
index 1517fff..1e75f1a 100755
--- a/t/t4019-diff-wserror.sh
+++ b/t/t4019-diff-wserror.sh
@@ -190,4 +190,13 @@ test_expect_success 'do not color trailing cr in context' '
'
+test_expect_success 'color new trailing blank lines' '
+ { echo a; echo b; echo; echo; } >x &&
+ git add x &&
+ { echo a; echo; echo; echo; echo; } >x &&
+ git diff --color x >output &&
+ cnt=$(grep "${blue_grep}" output | wc -l) &&
+ test $cnt = 2
+'
+
test_done
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 7/9] diff --whitespace=warn/error: obey blank-at-eof
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
The "diff --check" code used to conflate trailing-space whitespace error
class with this, but now we have a proper separate error class, we should
check it under blank-at-eof, not trailing-space.
The whitespace error is not about _having_ blank lines at end, but about
adding _new_ blank lines. To keep the message consistent with what is
given by "git apply", call whitespace_error_string() to generate it,
instead of using a hardcoded custom message.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 10 +++++++---
t/t4015-diff-whitespace.sh | 4 ++--
t/t4019-diff-wserror.sh | 2 +-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 1eddd59..a693d18 100644
--- a/diff.c
+++ b/diff.c
@@ -1650,10 +1650,14 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
ecb.priv = &data;
xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
- if ((data.ws_rule & WS_TRAILING_SPACE) &&
+ if ((data.ws_rule & WS_BLANK_AT_EOF) &&
data.trailing_blanks_start) {
- fprintf(o->file, "%s:%d: ends with blank lines.\n",
- data.filename, data.trailing_blanks_start);
+ static char *err;
+
+ if (!err)
+ err = whitespace_error_string(WS_BLANK_AT_EOF);
+ fprintf(o->file, "%s:%d: %s\n",
+ data.filename, data.trailing_blanks_start, err);
data.status = 1; /* report errors */
}
}
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index b1cbd36..a5d4461 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -335,10 +335,10 @@ test_expect_success 'line numbers in --check output are correct' '
'
-test_expect_success 'checkdiff detects trailing blank lines' '
+test_expect_success 'checkdiff detects new trailing blank lines (1)' '
echo "foo();" >x &&
echo "" >>x &&
- git diff --check | grep "ends with blank"
+ git diff --check | grep "new blank line"
'
test_expect_success 'checkdiff allows new blank lines' '
diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh
index 84a1fe3..1517fff 100755
--- a/t/t4019-diff-wserror.sh
+++ b/t/t4019-diff-wserror.sh
@@ -165,7 +165,7 @@ test_expect_success 'trailing empty lines (1)' '
rm -f .gitattributes &&
test_must_fail git diff --check >output &&
- grep "ends with blank lines." output &&
+ grep "new blank line at" output &&
grep "trailing whitespace" output
'
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 6/9] diff.c: the builtin_diff() deals with only two-file comparison
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
The combined diff is implemented in combine_diff() and fn_out_consume()
codepath never has to deal with anything but two-file comparison.
Drop nparents from the emit_callback structure and simplify the code.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 32 +++++++++-----------------------
1 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/diff.c b/diff.c
index 6fea3c0..1eddd59 100644
--- a/diff.c
+++ b/diff.c
@@ -489,7 +489,7 @@ typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
struct emit_callback {
struct xdiff_emit_state xm;
- int nparents, color_diff;
+ int color_diff;
unsigned ws_rule;
sane_truncate_fn truncate;
const char **label_path;
@@ -549,9 +549,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
emit_line(ecbdata->file, set, reset, line, len);
else {
/* Emit just the prefix, then the rest. */
- emit_line(ecbdata->file, set, reset, line, ecbdata->nparents);
- ws_check_emit(line + ecbdata->nparents,
- len - ecbdata->nparents, ecbdata->ws_rule,
+ emit_line(ecbdata->file, set, reset, line, 1);
+ ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
ecbdata->file, set, reset, ws);
}
}
@@ -576,7 +575,6 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u
static void fn_out_consume(void *priv, char *line, unsigned long len)
{
- int i;
int color;
struct emit_callback *ecbdata = priv;
const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
@@ -598,13 +596,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
}
- /* This is not really necessary for now because
- * this codepath only deals with two-way diffs.
- */
- for (i = 0; i < len && line[i] == '@'; i++)
- ;
- if (2 <= i && i < len && line[i] == ' ') {
- ecbdata->nparents = i - 1;
+ if (line[0] == '@') {
len = sane_truncate_line(ecbdata, line, len);
emit_line(ecbdata->file,
diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
@@ -614,15 +606,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
return;
}
- if (len < ecbdata->nparents) {
+ if (len < 1) {
emit_line(ecbdata->file, reset, reset, line, len);
return;
}
color = DIFF_PLAIN;
- if (ecbdata->diff_words && ecbdata->nparents != 1)
- /* fall back to normal diff */
- free_diff_words_data(ecbdata);
if (ecbdata->diff_words) {
if (line[0] == '-') {
diff_words_append(line, len,
@@ -641,13 +630,10 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
emit_line(ecbdata->file, plain, reset, line, len);
return;
}
- for (i = 0; i < ecbdata->nparents && len; i++) {
- if (line[i] == '-')
- color = DIFF_FILE_OLD;
- else if (line[i] == '+')
- color = DIFF_FILE_NEW;
- }
-
+ if (line[0] == '-')
+ color = DIFF_FILE_OLD;
+ else if (line[0] == '+')
+ color = DIFF_FILE_NEW;
if (color != DIFF_FILE_NEW) {
emit_line(ecbdata->file,
diff_get_color(ecbdata->color_diff, color),
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 5/9] apply --whitespace: warn blank but not necessarily empty lines at EOF
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
The whitespace error of adding blank lines at the end of file should
trigger if you added a non-empty line at the end, if the contents of the
line is full of whitespaces.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-apply.c | 6 ++++--
t/t4124-apply-ws-rule.sh | 13 +++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 37d3bc0..6662cc4 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1957,7 +1957,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
is_blank_context = 1;
break;
case ' ':
- if (plen && patch[1] == '\n')
+ if (plen && (ws_rule & WS_BLANK_AT_EOF) &&
+ ws_blank_line(patch + 1, plen, ws_rule))
is_blank_context = 1;
case '-':
memcpy(old, patch + 1, plen);
@@ -1985,7 +1986,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
(first == '+' ? 0 : LINE_COMMON));
new += added;
if (first == '+' &&
- added == 1 && new[-1] == '\n')
+ (ws_rule & WS_BLANK_AT_EOF) &&
+ ws_blank_line(patch + 1, plen, ws_rule))
added_blank_line = 1;
break;
case '@': case '\\':
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 89b71e1..b3c3b2c 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -215,4 +215,17 @@ test_expect_success 'blank at EOF with --whitespace=error' '
grep "new blank line at EOF" error
'
+test_expect_success 'blank but not empty at EOF' '
+ { echo a; echo b; echo c; } >one &&
+ git add one &&
+ echo " " >>one &&
+ cat one >expect &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ git apply --whitespace=warn patch 2>error &&
+ test_cmp expect one &&
+ grep "new blank line at EOF" error
+'
+
test_done
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 4/9] apply --whitespace=warn/error: diagnose blank at EOF
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
"git apply" strips new blank lines at EOF under --whitespace=fix option,
but neigher --whitespace=warn nor --whitespace=error paid any attention to
these errors.
Introduce a new whitespace error class, blank-at-eof, to make the
whitespace error handling more consistent.
The patch adds a new "linenr" field to the struct fragment in order to
record which line the hunk started in the input file, but this is needed
solely for reporting purposes. The detection of this class of whitespace
errors cannot be done while parsing a patch like we do for all the other
classes of whitespace errors. It instead has to wait until we find where
to apply the hunk, but at that point, we do not have an access to the
original line number in the input file anymore, hence the new field.
Depending on your point of view, this may be a bugfix that makes warn and
error in line with fix. Or you could call it a new feature. The line
between them is somewhat fuzzy in this case.
Strictly speaking, triggering more errors than before is a change in
behaviour that is not backward compatible, even though the reason for the
change is because the code was not checking for an error that it should
have. People who do not want added blank lines at EOF to trigger an error
can disable the new error class.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 2 ++
builtin-apply.c | 27 ++++++++++++++++++---------
cache.h | 3 ++-
t/t4124-apply-ws-rule.sh | 26 ++++++++++++++++++++++++++
ws.c | 6 ++++++
5 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 113d9d1..871384e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -389,6 +389,8 @@ core.whitespace::
error (enabled by default).
* `indent-with-non-tab` treats a line that is indented with 8 or more
space characters as an error (not enabled by default).
+* `blank-at-eof` treats blank lines added at the end of file as an error
+ (enabled by default).
* `cr-at-eol` treats a carriage-return at the end of line as
part of the line terminator, i.e. with it, `trailing-space`
does not trigger if the character before such a carriage-return
diff --git a/builtin-apply.c b/builtin-apply.c
index 80ddf55..37d3bc0 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -126,6 +126,7 @@ struct fragment {
const char *patch;
int size;
int rejected;
+ int linenr;
struct fragment *next;
};
@@ -1193,6 +1194,7 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc
int len;
fragment = xcalloc(1, sizeof(*fragment));
+ fragment->linenr = linenr;
len = parse_fragment(line, size, patch, fragment);
if (len <= 0)
die("corrupt patch at line %d", linenr);
@@ -2079,17 +2081,24 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
}
if (applied_pos >= 0) {
- if (ws_error_action == correct_ws_error &&
- new_blank_lines_at_end &&
- preimage.nr + applied_pos == img->nr) {
+ if (new_blank_lines_at_end &&
+ preimage.nr + applied_pos == img->nr &&
+ (ws_rule & WS_BLANK_AT_EOF) &&
+ ws_error_action != nowarn_ws_error) {
+ record_ws_error(WS_BLANK_AT_EOF, "+", 1, frag->linenr);
+ if (ws_error_action == correct_ws_error) {
+ while (new_blank_lines_at_end--)
+ remove_last_line(&postimage);
+ }
/*
- * If the patch application adds blank lines
- * at the end, and if the patch applies at the
- * end of the image, remove those added blank
- * lines.
+ * We would want to prevent write_out_results()
+ * from taking place in apply_patch() that follows
+ * the callchain led us here, which is:
+ * apply_patch->check_patch_list->check_patch->
+ * apply_data->apply_fragments->apply_one_fragment
*/
- while (new_blank_lines_at_end--)
- remove_last_line(&postimage);
+ if (ws_error_action == die_on_ws_error)
+ apply = 0;
}
/*
diff --git a/cache.h b/cache.h
index 099a32e..7152fea 100644
--- a/cache.h
+++ b/cache.h
@@ -845,7 +845,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
#define WS_SPACE_BEFORE_TAB 02
#define WS_INDENT_WITH_NON_TAB 04
#define WS_CR_AT_EOL 010
-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
+#define WS_BLANK_AT_EOF 020
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)
extern unsigned whitespace_rule_cfg;
extern unsigned whitespace_rule(const char *);
extern unsigned parse_whitespace_rule(const char *);
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index ba2b7f9..89b71e1 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -189,4 +189,30 @@ test_expect_success 'blank at EOF with --whitespace=fix (3)' '
test_cmp expect one
'
+test_expect_success 'blank at EOF with --whitespace=warn' '
+ { echo a; echo b; echo c; } >one &&
+ git add one &&
+ echo >>one &&
+ cat one >expect &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ git apply --whitespace=warn patch 2>error &&
+ test_cmp expect one &&
+ grep "new blank line at EOF" error
+'
+
+test_expect_success 'blank at EOF with --whitespace=error' '
+ { echo a; echo b; echo c; } >one &&
+ git add one &&
+ cat one >expect &&
+ echo >>one &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ test_must_fail git apply --whitespace=error patch 2>error &&
+ test_cmp expect one &&
+ grep "new blank line at EOF" error
+'
+
test_done
diff --git a/ws.c b/ws.c
index 7a7ff13..d56636b 100644
--- a/ws.c
+++ b/ws.c
@@ -15,6 +15,7 @@ static struct whitespace_rule {
{ "space-before-tab", WS_SPACE_BEFORE_TAB },
{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
{ "cr-at-eol", WS_CR_AT_EOL },
+ { "blank-at-eof", WS_BLANK_AT_EOF },
};
unsigned parse_whitespace_rule(const char *string)
@@ -113,6 +114,11 @@ char *whitespace_error_string(unsigned ws)
strbuf_addstr(&err, ", ");
strbuf_addstr(&err, "indent with spaces");
}
+ if (ws & WS_BLANK_AT_EOF) {
+ if (err.len)
+ strbuf_addstr(&err, ", ");
+ strbuf_addstr(&err, "new blank line at EOF");
+ }
return strbuf_detach(&err, NULL);
}
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 3/9] apply.c: split check_whitespace() into two
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
This splits the logic to record the presence of whitespace errors out of
the check_whitespace() function, which checks and then records. The new
function, record_ws_error(), can be used by the blank-at-eof check that
does not use ws_check() logic to report its findings in the same output
format.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-apply.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index c5e4048..80ddf55 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1055,23 +1055,29 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
return -1;
}
-static void check_whitespace(const char *line, int len, unsigned ws_rule)
+static void record_ws_error(unsigned result, const char *line, int len, int linenr)
{
char *err;
- unsigned result = ws_check(line + 1, len - 1, ws_rule);
+
if (!result)
return;
whitespace_error++;
if (squelch_whitespace_errors &&
squelch_whitespace_errors < whitespace_error)
- ;
- else {
- err = whitespace_error_string(result);
- fprintf(stderr, "%s:%d: %s.\n%.*s\n",
- patch_input_file, linenr, err, len - 2, line + 1);
- free(err);
- }
+ return;
+
+ err = whitespace_error_string(result);
+ fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+ patch_input_file, linenr, err, len, line);
+ free(err);
+}
+
+static void check_whitespace(const char *line, int len, unsigned ws_rule)
+{
+ unsigned result = ws_check(line + 1, len - 1, ws_rule);
+
+ record_ws_error(result, line + 1, len - 2, linenr);
}
/*
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
The command tries to strip blank lines at the end of the file added by a
patch. However, if the original ends with blank lines, often the patch
hunk ends like this:
@@ -l,5 +m,7 @@$
_context$
_context$
-deleted$
+$
+$
+$
_$
_$
where _ stands for SP and $ shows a end-of-line. This example patch adds
three trailing blank lines, but the code fails to notice it, because it
only pays attention to added blank lines at the very end of the hunk. In
this example, the three added blank lines do not appear textually at the
end in the patch, even though you can see that they are indeed added at
the end, if you rearrange the diff like this:
@@ -l,5 +m,7 @@$
_context$
_context$
-deleted$
_$
_$
+$
+$
+$
Fix this by not resetting the number of (candidate) added blank lines at
the end when the loop sees a context line that is empty.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-apply.c | 6 ++++++
t/t4124-apply-ws-rule.sh | 12 ++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 5b5bde4..c5e4048 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1913,6 +1913,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
int len = linelen(patch, size);
int plen, added;
int added_blank_line = 0;
+ int is_blank_context = 0;
if (!len)
break;
@@ -1945,8 +1946,11 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
*new++ = '\n';
add_line_info(&preimage, "\n", 1, LINE_COMMON);
add_line_info(&postimage, "\n", 1, LINE_COMMON);
+ is_blank_context = 1;
break;
case ' ':
+ if (plen && patch[1] == '\n')
+ is_blank_context = 1;
case '-':
memcpy(old, patch + 1, plen);
add_line_info(&preimage, old, plen,
@@ -1986,6 +1990,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
}
if (added_blank_line)
new_blank_lines_at_end++;
+ else if (is_blank_context)
+ ;
else
new_blank_lines_at_end = 0;
patch += len;
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 6898722..ba2b7f9 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -177,4 +177,16 @@ test_expect_success 'blank at EOF with --whitespace=fix (2)' '
test_cmp expect one
'
+test_expect_success 'blank at EOF with --whitespace=fix (3)' '
+ { echo a; echo b; echo; } >one &&
+ git add one &&
+ { echo a; echo c; echo; } >expect &&
+ { cat expect; echo; echo; } >one &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ git apply --whitespace=fix patch &&
+ test_cmp expect one
+'
+
test_done
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 1/9] apply --whitespace=fix: fix handling of blank lines at the eof
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>
b94f2ed (builtin-apply.c: make it more line oriented, 2008-01-26) broke
the logic used to detect if a hunk adds blank lines at the end of the
file. With the new code after that commit:
- img holds the contents of the file that the hunk is being applied to;
- preimage has the lines the hunk expects to be in img; and
- postimage has the lines the hunk wants to update the part in img that
corresponds to preimage with.
and we need to compare if the last line of preimage (not postimage)
matches the last line of img to see if the hunk applies at the end of the
file.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-apply.c | 2 +-
t/t4124-apply-ws-rule.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 7a1ff04..5b5bde4 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2069,7 +2069,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
if (applied_pos >= 0) {
if (ws_error_action == correct_ws_error &&
new_blank_lines_at_end &&
- postimage.nr + applied_pos == img->nr) {
+ preimage.nr + applied_pos == img->nr) {
/*
* If the patch application adds blank lines
* at the end, and if the patch applies at the
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index f83322e..6898722 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -148,4 +148,33 @@ do
done
done
+
+test_expect_success 'blank at EOF with --whitespace=fix (1)' '
+ : these can fail depending on what we did before
+ git config --unset core.whitespace
+ rm -f .gitattributes
+
+ { echo a; echo b; echo c; } >one &&
+ git add one &&
+ { echo a; echo b; echo c; } >expect &&
+ { cat expect; echo; } >one &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ git apply --whitespace=fix patch &&
+ test_cmp expect one
+'
+
+test_expect_success 'blank at EOF with --whitespace=fix (2)' '
+ { echo a; echo b; echo c; } >one &&
+ git add one &&
+ { echo a; echo c; } >expect &&
+ { cat expect; echo; echo; } >one &&
+ git diff -- one >patch &&
+
+ git checkout one &&
+ git apply --whitespace=fix patch &&
+ test_cmp expect one
+'
+
test_done
--
1.6.4.2.313.g0425f
^ permalink raw reply related
* [PATCH 0/9] War on blank-at-eof
From: Junio C Hamano @ 2009-09-04 10:55 UTC (permalink / raw)
To: git
We had quite inconsistent handing of patches that add new blank lines at
the end of file, and this miniseries is about fixing it.
Patch 1 is a fix to an ancient bug introduced by v1.5.5-rc0~156^2~11.
Patch 2 fixes a bug that is even older---I suspect it dates back to the
very first change that introduced the feature, but I did not bother to
dig.
Patch 4 (Patch 3 is a preliminary refactoring used by it) is about the
discrepancy between "--whitespace=fix" and "--whitespace=warn". The
blank-at-eof error was silently fixed but never diagnosed, which has
been one of the long-standing itch of mine to fix.
Patch 5 corrects the definition of blank-at-eof. If a patch adds an
non-empty line that consists solely of whitespaces at the end of file, we
should diagnose and strip it just line a new empty line. After all, both
are blank lines.
Patch 6 is a simple code reduction I noticed while preparing this series;
it can be a standalone patch, but it is obvious enough to be here.
Patches 7 and 8 address "git diff --check", which had roughly the same
logic as the --whitespace=fix. It shared the same problems the earlier
parts of the series fixed for "git apply".
Patch 9 is about "diff --color" to paint blank-at-eof as error, which we
did not do so far because it was too cumbersome. This has been another
one of the long-standing itch of mine to fix.
The series applies to v1.6.0.6-87-g82d97da; merging the result to 'master'
needs some conflict resolution.
1 apply --whitespace=fix: fix handling of blank lines at the eof
2 apply --whitespace=fix: detect new blank lines at eof correctly
3 apply.c: split check_whitespace() into two
4 apply --whitespace=warn/error: diagnose blank at EOF
5 apply --whitespace: warn blank but not necessarily empty lines at EOF
6 diff.c: the builtin_diff() deals with only two-file comparison
7 diff --whitespace=warn/error: obey blank-at-eof
8 diff --whitespace=warn/error: fix blank-at-eof check
9 diff --color: color blank-at-eof
Documentation/config.txt | 2 +
builtin-apply.c | 61 +++++++++++++++-------
cache.h | 3 +-
diff.c | 119 +++++++++++++++++++++++++++++---------------
t/t4015-diff-whitespace.sh | 11 +++-
t/t4019-diff-wserror.sh | 11 ++++-
t/t4124-apply-ws-rule.sh | 80 +++++++++++++++++++++++++++++
ws.c | 6 ++
8 files changed, 230 insertions(+), 63 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Junio C Hamano @ 2009-09-04 10:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Daniel Barkalow, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0909041232500.4605@intel-tinevez-2-302>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The special case is "http://" and "https://" which might indicate foreign
> VCS repositories.
>
> In all other cases, I am afraid that you are complicating the glove.
>
> Remember: the whole purpose of the "foreign VCS" helpers is user
> convenience.
Sorry, but you completely lost me here.
Here are two URLs that follows your "user convenience" principle.
http://example.xz/repos/frotz.git/
http://example.xz/repo/frotz/trunk/
How do you tell, without relying on .git and trunk, the former is a git
repository and wants the dumb walker transport to fetch from, while the
latter is probably a svn and you would either use "svn checkout", or use
"git clone" on it via the svn helper?
Well, you don't.
One possible "convenient user interface" would be to say
svn+http://example.xz/repo/frotz/trunk/
(or use :: instead of + as the helper-name separator, as we agreed not to
decide on it)
This would give us
(1) it is clear that it literally is what you would give to git and
trigger the svn helper; and
(2) to people who know how canonical git URLs with foreign helper are
spelled, it also is clear that you can use "svn checkout" on
everything after "svn+" in it.
A corollary to this is that you can also use "git svn init" on it.
Compared to that, if you do not have any such prefix, how would that be
more convenient to the users?
Or perhaps you have an alternative in mind that is more convenient for the
users and that is not "use identically looking http://... for both", but
you are being unnecessarily cryptic by not spelling out what it is.
^ permalink raw reply
* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Sverre Rabbelier @ 2009-09-04 10:47 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Nanako Shiraishi, Junio C Hamano, git
In-Reply-To: <20090904172345.6117@nanako3.lavabit.com>
Heya,
On Fri, Sep 4, 2009 at 10:23, Nanako Shiraishi<nanako3@lavabit.com> wrote:
> http://thread.gmane.org/gmane.comp.version-control.git/127121/focus=127520
I don't see anything in that thread that convinces me why this is the
better solution. Unless I'm reading it wrong Junio said "so this is
how you're going to do it", and Daniel said "yup".
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 4/8] Allow fetch to modify refs
From: Johannes Schindelin @ 2009-09-04 10:46 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909032213260.28290@iabervon.org>
Hi,
On Thu, 3 Sep 2009, Daniel Barkalow wrote:
> + /**
> + * Fetch the objects for the given refs. Note that this gets
> + * an array, and should ignore the list structure.
This is not clear at all. You should rather say "[...] and should not
look at, or set, the 'next' member of the refs".
> + *
> + * If the transport did not get hashes for refs in
> + * get_refs_list(), it should set the old_sha1 fields in the
> + * provided refs now.
Not the "new_sha1"?
> + **/
> + int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
> +
> [...]
> + /** get_refs_list(), fetch(), and push_refs() can keep
The "/**" wants to have a line to itself.
> + * resources (such as a connection) reserved for futher
> + * use. disconnect() releases these resources.
> + **/
> int (*disconnect)(struct transport *connection);
> char *pack_lockfile;
> signed verbose : 2;
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/8] Use a clearer style to issue commands to remote helpers
From: Johannes Schindelin @ 2009-09-04 10:40 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909032213200.28290@iabervon.org>
Hi,
On Thu, 3 Sep 2009, Daniel Barkalow wrote:
> This style is overkill for some commands, but it's worthwhile to use
> the same style to issue all commands, and it's useful to avoid
> open-coding string lengths.
Why do I have to study the patch to find out what "this style" is? And
why do you not even _try_ to convince people that "it's worthwhile", say,
by giving some example?
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 4684877..b1ea7e6 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)
> die("Unable to run helper: git %s", helper->argv[0]);
> data->helper = helper;
>
> - write_in_full(data->helper->in, "capabilities\n", 13);
> + strbuf_addstr(&buf, "capabilities\n");
> + write_in_full(helper->in, buf.buf, buf.len);
> + strbuf_reset(&buf);
> +
If you use that paradigm more often, why not rather introduce something
like
void strbuf_flush(struct strbuf *buf, int fd) {
write_in_full(fd, buf->buf, buf->len);
strbuf_reset(buf);
}
instead of violating the DRY principle?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Johannes Schindelin @ 2009-09-04 10:34 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909032213180.28290@iabervon.org>
Hi,
On Thu, 3 Sep 2009, Daniel Barkalow wrote:
> Instead of trying to make http://, https://, and ftp:// URLs indicative
> of some sort of pattern of transport helper usage, make them a special
> case which runs the "curl" helper, and leave the mechanism by which
> arbitrary helpers will be chosen entirely to future work.
I have to admit that this does not convince me at all.
The special case is "http://" and "https://" which might indicate foreign
VCS repositories.
In all other cases, I am afraid that you are complicating the glove.
Remember: the whole purpose of the "foreign VCS" helpers is user
convenience.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-completion.bash: prevent 'git help' from searching for git repository
From: Johannes Schindelin @ 2009-09-04 10:32 UTC (permalink / raw)
To: Rogan Dawes; +Cc: Sverre Rabbelier, Gerrit Pape, Junio C Hamano, git
In-Reply-To: <4AA0E142.4080105@dawes.za.net>
Hi,
On Fri, 4 Sep 2009, Rogan Dawes wrote:
> Sverre Rabbelier wrote:
>
> > On Wed, Sep 2, 2009 at 11:58, Gerrit Pape<pape@smarden.org> wrote:
> >> + for i in $(git --git-dir=/nonexistent help -a|egrep '^ ')
> >
> > Wouldn't implementing "git --no-git-dir" be more appropriate?
>
> Or documenting which git commands do/don't require a git dir at all?
This patch is not about documentation, but about preventing the
auto-completion from trying to discover a Git repository (to prevent
auto-mounting; although I wonder why you would run Git there if you do not
want to auto-mount).
> I assume that documenting those that don't would be better than
> documenting those that do . . .
It's not as easy as that: some commands, such as "ls-remote" do _not_
require one, but they take it into account (think "git ls-remote origin").
Other commands, such as "archive", have modes in which they _need_ a
repository, and other modes where they do not even look for one.
"git help -a" seems to be similar to the latter modes of "archive".
> And by documenting, I mean in the code, so that the code can DTRT.
>
> Otherwise, having this switch lets people shoot themselves in the foot,
> I'd think.
Git offers plenty of opportunity to shoot yourself in the foot (and it
does not help that we are introducing user-unfriendly constructs like the
current form of the foreign VCS helpers with more such opportunities,
either), but for the love of God, I cannot find how "this switch" lets
people shoot themselves in the foot here.
Ciao,
Dscho
^ permalink raw reply
* Re: Issue 323 in msysgit: Can't clone over http
From: Junio C Hamano @ 2009-09-04 10:25 UTC (permalink / raw)
To: git, Tay Ray Chuan; +Cc: msysgit
In-Reply-To: <0016e6470f36315b8a0472bc75a8@google.com>
codesite-noreply@google.com writes:
> Status: New
> Owner: ----
>
> New issue 323 by bjelli: Can't clone over http
> http://code.google.com/p/msysgit/issues/detail?id=323
>
> What steps will reproduce the problem?
> 1.Install Git-1.6.4-preview20090730.exe
> 2.Clone exsiting repository http://github.com/tekkub/addontemplate.git
This does not seem to be an msysgit issue. Even on a Linux host, v1.6.2.5
seems to work Ok but 'maint', 'master', nor 'next' does not clone this one
correctly.
> Output:
> got 2c8851d269d51676b8c626e63991ee68a6f5d578
> walk 2c8851d269d51676b8c626e63991ee68a6f5d578
> got 758419d18ad255c3417ca341c6e12c6ca1aa203e
> got fa8a1ec5a791c245789f70e90a844f2b9a275991
> walk fa8a1ec5a791c245789f70e90a844f2b9a275991
> got e884a228df0e08e0f862edab6012d8407907ab48
> got f7ea166470af2538a6a19642f8c45213bac7bd40
> got 6100842656e95bf50f2c6f3ff6e997bcbe2474cc
> got 445c0ea7c7193f6fcb42b32db50104926d328322
> got a44d6309e48622590b2780f96bed371122db6b71
> got 3ecefa3f04f394f64f8fe7be14ac20e69f2f2c18
> Getting alternates list for http://github.com/tekkub/addontemplate.git
> Getting pack list for http://github.com/tekkub/addontemplate.git
> error: Unable to verify pack 382c25c935b744e909c749532578112d72a4aff9 is
> available
> error: Unable to find 0a41ac04d56ccc96491989dc71d9875cd804fc6b under
> http://github.com/tekkub/addontemplate.git
> Cannot obtain needed blob 0a41ac04d56ccc96491989dc71d9875cd804fc6b
> while processing commit fa8a1ec5a791c245789f70e90a844f2b9a275991.
> fatal: Fetch failed.
>
> What version of the product are you using? On what operating system?
> Git-1.6.4-preview20090730.exe
> Windows XP
>
> Please provide any additional information below.
> Defect was discussed on the github support board here:
> http://support.github.com/discussions/repos/957-cant-clone-over-http-or-git
^ 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