* Re: Deleted folder keeps showing up?
From: Benjamin Buch @ 2009-09-04 8:18 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4AA0C4A2.5040405@drmicha.warpmail.net>
Hi Michael,
thank you for your answer.
Strangely, I can't reproduce the error today.
As I did quite a lot branching and merging yesterday (learning git),
I can't remember the steps that led to the error.
And it doesn't have to be an error though -
perhaps it was just my clouded perception due to the lots of branches.
But I will keep an eye on this and see if it happens again.
Sorry for bothering,
- benjamin
Am 04.09.2009 um 09:41 schrieb Michael J Gruber:
> Benjamin Buch venit, vidit, dixit 03.09.2009 18:59:
>> I made a branch and deleted a folder there with git rm -rf
>> foldername.
>> So now i have to branches, A with the folder and B without the
>> folder.
>>
>> I'm on B, the folder is not there.
>> Then I check out A, the folder shows up like it should.
>> When I check out B again, the folder is still there.
>>
>> A git rm -rf folder gives me:
>>
>> fatal: pathspec 'folder/' did not match any files
>>
>> , so git is not tracking the folder.
>>
>> I can rm -rf the filder without git and start the whole game from the
>> beginning,
>> but there has to be a better way?
>>
>> Strange enough this happens just to two folders I removed,
>> with others there is no problem.
>
> What does "git status" say when you've checked out B? Could some
> contents of folder/ possibly be being ignored (.git/info/excludes
> etc.)?
>
> Michael
> --
> 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
* Re: Deleted folder keeps showing up?
From: Jeff King @ 2009-09-04 8:27 UTC (permalink / raw)
To: Benjamin Buch; +Cc: git
In-Reply-To: <34230C98-81B8-4DC8-846F-8B6FA2A022DA@gmx.de>
On Thu, Sep 03, 2009 at 06:59:18PM +0200, Benjamin Buch wrote:
> I made a branch and deleted a folder there with git rm -rf foldername.
> So now i have to branches, A with the folder and B without the folder.
>
> I'm on B, the folder is not there.
> Then I check out A, the folder shows up like it should.
> When I check out B again, the folder is still there.
Is there anything in the folder, like untracked files generated by your
build process? Remember that git tracks full paths, not directories. So
you never actually "deleted a folder" but rather deleted all of the
paths inside that folder.
When you switch to branch A, git creates the folder, because it contains
tracked files. When you switch back to branch B, git will remove the
tracked files, and will remove the directory _only_ if it is then empty.
Anything else would mean deleting your untracked files, which may be
precious.
You mentioned in a later email that you were having trouble reproducing
the issue. Try:
$ git checkout A
$ make ;# or whatever your build process is, or
;# normal work or whatever
$ git checkout B
and see if that reproduces it.
-Peff
^ permalink raw reply
* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: SZEDER Gábor @ 2009-09-04 8:32 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090902081917.GA5447@coredump.intra.peff.net>
[Oops, I 've just noticed that my reply to Jeff didn't made it to the
git list, because I hit 'reply' instead of 'reply to all'...]
Hi Jeff,
thanks for your quick reply.
On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:
> On Wed, Sep 02, 2009 at 10:03:05AM +0200, SZEDER Gábor wrote:
>
> > As the subject says, 'git add -u' does not work from an untracked
> > subdir, because it doesn't add modified files to the index. The
> > following script reproduces the issue:
> >
> > mkdir repo
> > cd repo
> > git init
> > echo 1 >foo
> > git add foo
> > git commit -m first
> > echo 2 >foo
> > mkdir untracked_subdir
> > cd untracked_subdir
> > git add -u
> > git diff
> >
> > It worked in the initial 'git add -u' implementation (dfdac5d,
> > git-add
> > -u: match the index with working tree, 2007-04-20), but 2ed2c222
> > (git-add -u paths... now works from subdirectory, 2007-08-16)
> > broke it
> > later, and is broken ever since.
>
> It is not just untracked subdirs. Try:
>
> mkdir repo && cd repo && git init
> echo 1 >foo
> mkdir subdir
> echo 1 >subdir/bar
> git add . && git commit -m first
> echo 2 >foo
> echo 2 >subdir/bar
> cd subdir
> git add -u
> git diff ;# still shows foo/1 in index
> git diff --cached ;# shows subdir/bar was updated
>
> While I have sometimes found the behavior a bit annoying[1], I
> always
> assumed that was the intended behavior.
>
> And indeed, in modern builtin-add.c, we find this:
>
> if ((addremove || take_worktree_changes) && !argc) {
> static const char *here[2] = { ".", NULL };
> argc = 1;
> argv = here;
> }
>
> which seems pretty explicit.
Since then I looked at the man page (I should have done that right
away ;), and it says under the description of -u that "If no paths are
specified, all tracked files in the current directory and its
subdirectories are updated." So this is indeed the intended
behaviour, but I was just not aware of it. Oh well, sorry for the
noise.
> [1] I would prefer "git add -u ." to add only the current directory,
> and
> "git add -u" to touch everything. But then, I am one of the people
> who
> turn off status.relativepaths, so I think I may be in the minority
> in
> always wanting to think of the project as a whole.
I don't really know which would I prefer.
I was updating some Javadoc documentation in Eclipse, and checking the
generated docs in terminal, deep down in an untracked subdir, and
performed some 'add -u ; commit --amend' from there (and was rather
surprised after the fifth amend to see all the changes still in the
worktree). Doing perform the desired add -u from there I should have
run 'git add -u ../../../../../..', what doesn't seem very convenient.
But since this was the first time I've done that since 2007-08-16, I
guess it's not a very common use case.
Gábor
^ permalink raw reply
* Re: Deleted folder keeps showing up?
From: Junio C Hamano @ 2009-09-04 8:36 UTC (permalink / raw)
To: Benjamin Buch; +Cc: Michael J Gruber, git
In-Reply-To: <8E56979B-5D85-4844-A492-8149EE9E9B2F@gmx.de>
Benjamin Buch <benni.buch@gmx.de> writes:
> Strangely, I can't reproduce the error today.
If a branch has dir/file tracked, and another branch does not have
anything tracked in dir/ directory at all, then switching from the former
branch to the latter can remove dir/ only when you do not have any
untracked files in there when you switch. Otherwise dir/ must stay
behind to keep the untracked files.
You can see it by a simple experiment.
$ rm -fr trial
$ mkdir trial
$ cd trial
$ git init
$ >elif
$ git commit -m initial
$ git branch lacksdir
$ mkdir dir
$ >dir/file
$ git add dir/file
$ git commit -m add-dir-file
Now, after this set-up, your 'master' has dir/file and 'lacksdir' does
not have anything tracked in dir/ directory.
Observe:
$ git checkout lacksdir
$ find ??*
elif
$ git checkout master
$ find ??*
dir
dir/file
elif
$ >dir/garbage
$ git checkout lacksdir
$ find ??*
dir
dir/garbage
elif
If switching to 'lacksdir' removed the dir/ directory, whatever was in the
untracked file dir/garbage will be lost. In the above exercise, I named
it garbage, so a casual reader might get a false impression that it should
be thrown away, but in real life workflow, it often happens that
(1) you start doing some interesting experimental changes, while on
'master';
(2) you realize that this change does not belong to 'master', but belongs
to some other branch, perhaps 'lacksdir';
(3) you switch to the branch, to keep working.
Remember that, in git, your uncommitted changes to the index and the work
tree do not belong to the branch. They try to follow you across branch
switching. Since untracked new files are something potentially you might
want to add after branch switching, we do not remove them. And because we
choose not to remove dir/file, even though the commit at the tip of the
lacksdir branch does not have anything tracked in dir/ directory, we
cannot remove it from the work tree.
^ permalink raw reply
* Re: [PATCHv2] git-config: Parse config files leniently
From: Junio C Hamano @ 2009-09-04 8:40 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4AA0BE30.1030408@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>> *2* Worse yet, the parsing of branch.autosetuprebase is part of the
>> default_config and commands that do not have anything to do with new
>> branch creation will fail with the current setup.
>
> Thanks for the thorough explanations. Especially *2* makes me think that
> quite some restructuring would be necessary in order to "do it right".
We need to distinguish at least the "syntax" and "semantics" errors, i.e.
not necessarily "do it right", but "how your patch should have looked
like".
I think a slightly hacky but practical workaround then becomes possible.
We can have a config callback function to parse only core.editor (and
perhaps some other very minimum set of variables needed to launch the
editor on the right config file) extremely loosely, i.e. not even calling
git_config_string() to complain about error-nonbool. You can use the
callback _only_ from the ACTION_EDIT codepath in builtin-config.c (which
currently uses git_default_config and errors out when some uninteresting
configuration variables have semantic errors).
That would get rid of the issue that the configuration mechanism triggers
semantic errors for unrelated variables, and would give us a usable
recovery editor, no?
^ permalink raw reply
* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special case
From: Mike Ralphson @ 2009-09-04 9:04 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909032213180.28290@iabervon.org>
2009/9/4 Daniel Barkalow <barkalow@iabervon.org>:
> 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.
> - PROGRAMS += git-remote-http$X $(CURL_SYNONYMS) git-http-fetch$X
> + PROGRAMS += git-remote-curl$X git-http-fetch$X
I think .gitignore would need to be updated again with the added and
removed executables?
Mike
^ permalink raw reply
* [PATCH 1/2] git: add new option --no-git-dir
From: Gerrit Pape @ 2009-09-04 9:29 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, git
In-Reply-To: <fabb9a1e0909020447p212594cake8c6fe3a43b667ec@mail.gmail.com>
This commit adds the --no-git-dir option to the git program. Setting
this option prevents the git program from searching for a path to a git
repository, which can be useful for commands that do not require one.
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
Documentation/git.txt | 6 +++++-
git.c | 6 +++++-
setup.c | 2 ++
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index ad44cac..6327203 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -11,7 +11,7 @@ SYNOPSIS
[verse]
'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path]
[-p|--paginate|--no-pager]
- [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]
+ [--bare] [--git-dir=GIT_DIR|--no-git-dir] [--work-tree=GIT_WORK_TREE]
[--help] COMMAND [ARGS]
DESCRIPTION
@@ -212,6 +212,10 @@ help ...`.
setting the GIT_DIR environment variable. It can be an absolute
path or relative path to current working directory.
+--no-git-dir::
+ Do not set a path to a repository, and do not try to find one.
+ Setting this option is equivalent to setting --git-dir="".
+
--work-tree=<path>::
Set the path to the working tree. The value will not be
used in combination with repositories found automatically in
diff --git a/git.c b/git.c
index 0b22595..8e060b9 100644
--- a/git.c
+++ b/git.c
@@ -5,7 +5,7 @@
#include "run-command.h"
const char git_usage_string[] =
- "git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+ "git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR|--no-git-dir] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
const char git_more_info_string[] =
"See 'git help COMMAND' for more information on a specific command.";
@@ -99,6 +99,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
if (envchanged)
*envchanged = 1;
+ } else if (!strcmp(cmd, "--no-git-dir")) {
+ setenv(GIT_DIR_ENVIRONMENT, "", 1);
+ if (envchanged)
+ *envchanged = 1;
} else if (!strcmp(cmd, "--work-tree")) {
if (*argc < 2) {
fprintf(stderr, "No directory given for --work-tree.\n" );
diff --git a/setup.c b/setup.c
index e3781b6..ee9be6e 100644
--- a/setup.c
+++ b/setup.c
@@ -335,6 +335,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
*nongit_ok = 1;
return NULL;
}
+ if (!*gitdirenv)
+ die("This command requires a git repository");
die("Not a git repository: '%s'", gitdirenv);
}
--
1.6.0.3
^ permalink raw reply related
* [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Gerrit Pape @ 2009-09-04 9:29 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, git
In-Reply-To: <fabb9a1e0909020447p212594cake8c6fe3a43b667ec@mail.gmail.com>
On 'git <TAB><TAB>' the bash completion runs 'git help -a'. Since 'git
help' actually doesn't need to be run inside a git repository, this
commit uses the --no-git-dir option to prevent it from searching a git
directory. Unnecessary searching for a git directory can be annoying in
auto-mount environments.
The annoying behavior and suggested fix has been reported by Vincent
Danjean through
http://bugs.debian.org/539273
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bf688e1..a55e3cd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -500,7 +500,7 @@ __git_all_commands ()
return
fi
local i IFS=" "$'\n'
- for i in $(git help -a|egrep '^ ')
+ for i in $(git --no-git-dir help -a|egrep '^ ')
do
case $i in
*--*) : helper pattern;;
--
1.6.0.3
^ permalink raw reply related
* Re: [PATCH] git-completion.bash: prevent 'git help' from searching for git repository
From: Rogan Dawes @ 2009-09-04 9:43 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Gerrit Pape, Junio C Hamano, git
In-Reply-To: <fabb9a1e0909020447p212594cake8c6fe3a43b667ec@mail.gmail.com>
Sverre Rabbelier wrote:
> Heya,
>
> 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?
I assume that documenting those that don't would be better than
documenting those that do . . .
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.
Rogan
^ permalink raw reply
* Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Junio C Hamano @ 2009-09-04 9:57 UTC (permalink / raw)
To: Gerrit Pape; +Cc: Sverre Rabbelier, git
In-Reply-To: <20090904092929.23208.qmail@00cf3567a0e8b4.315fe32.mid.smarden.org>
Gerrit Pape <pape@smarden.org> writes:
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index bf688e1..a55e3cd 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -500,7 +500,7 @@ __git_all_commands ()
> return
> fi
> local i IFS=" "$'\n'
> - for i in $(git help -a|egrep '^ ')
> + for i in $(git --no-git-dir help -a|egrep '^ ')
Thanks.
What the --no-git-dir option actually does is "pretend that cwd is the git
directory but do not worry if it is not", which is different from "there
is no git directory, so do not barf as long as you do not need to access
git-dir". The latter is what the name implies, and also additionally it
implies "but please do barf if you ever need to access something from git
directory." I do not know if that holds true with your Patch 1/2, and I
am a bit too tired to check.
Besides, "git --git-dir=." is shorter to type, and it is equally magical
that the user has to know about it anyway. Hopefully, most of the time
the end user would not have to use it directly, and the only demonstrated
use case is here in this completion script.
Would it be an option to chuck the Patch 1/2 at least for now and instead
say "git --git-dir=. help -a" here in this patch?
^ permalink raw reply
* Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Johannes Schindelin @ 2009-09-04 10:22 UTC (permalink / raw)
To: Gerrit Pape; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <20090904092929.23208.qmail@00cf3567a0e8b4.315fe32.mid.smarden.org>
Hi,
On Fri, 4 Sep 2009, Gerrit Pape wrote:
> On 'git <TAB><TAB>' the bash completion runs 'git help -a'.
Correct me if I am wrong, but does "git help -a" not list aliases? If it
does, "git help" must search for the Git repository.
If it does not, then "git help" needs fixing, not the completions. I.e.
something like this:
-- snipsnap --
[PATCH] git help -a: do not look for a repository
<all the acknowledgements go here>
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();
^ permalink raw reply related
* 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
* 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: [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 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 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 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 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
* [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
* [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 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 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 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 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 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
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