Git development
 help / color / mirror / Atom feed
* Re: [WIP PATCH] Add 'git fast-export', the sister of 'git fast-import'
From: Nguyen Thai Ngoc Duy @ 2007-11-24 14:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711231428350.27959@racer.site>

On Nov 23, 2007 9:31 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Fri, 23 Nov 2007, Nguyen Thai Ngoc Duy wrote:
>
> > On Nov 21, 2007 10:40 AM, Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > [WIP: this does not handle tags yet, and it lacks a test script
> > >  as well as documentation.]
> > >
> > > This program dumps (parts of) a git repository in the format that
> > > fast-import understands.
> > >
> > > For clarity's sake, it does not use the 'inline' method of specifying
> > > blobs in the commits, but builds the blobs before building the commits.B
> > >
> > > ---
> > >         I am way too tired now to continue, but maybe someone else wants
> > >         to pick up the ball.
> >
> > Well, I would better be back on setup_git_directory() than picking up
> > the ball.
>
> Concur.
>
> > I have a suggestion though. git-fast-export and git-fast-import should
> > support bundle.
>
> I think this is not what fast-export and fast-import are about.  They use
> an easy to generate, and easy to edit, format.
>
> Bundles are optimised transport mechanisms for sneaker net.  They are not
> to be meant to be easy to edit, but as small as possible.
>
> > Bundle is very handy for transferring a bunch of commits, but it does
> > not (cannot?) hold tags and branches.
>
> But they can!  Nothing prevents you from calling
>
>         git bundle create a1.bundle refs/tags/v1.0.0 refs/heads/next
>
> (At least this is the idea, haven't tested yet).

It can store commits and heavy tags, but it won't restore tags to
refs/tags or advance branches.

-- 
Duy

^ permalink raw reply

* [PATCH] git-svn now reads settings even if called in subdirectory
From: Gustaf Hendeby @ 2007-11-24 13:47 UTC (permalink / raw)
  To: git; +Cc: gitster, Gustaf Hendeby

Previously, git-svn first read the .git/config file for settings as if
current working directory was the repository top-directory, and after
that made sure to cd into top-directory.  The result was a silent
failur to read configuration settings.  This patch changes the order
these two things are done.

Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>

---

This fix seems a bit too obvious, which makes me think I missed
something conceptual here.  However, the patch passes "make test" and
"make full-svn-test", and the problems I had with the svn.authorsfile
setting not being honored while fetching from a subdirectory are gone.

 git-svn.perl |   35 ++++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 43e1591..640a45a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -193,23 +193,6 @@ for (my $i = 0; $i < @ARGV; $i++) {
 	}
 };
 
-my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
-
-read_repo_config(\%opts);
-Getopt::Long::Configure('pass_through') if ($cmd && $cmd eq 'log');
-my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
-                    'minimize-connections' => \$Git::SVN::Migration::_minimize,
-                    'id|i=s' => \$Git::SVN::default_ref_id,
-                    'svn-remote|remote|R=s' => sub {
-                       $Git::SVN::no_reuse_existing = 1;
-                       $Git::SVN::default_repo_id = $_[1] });
-exit 1 if (!$rv && $cmd && $cmd ne 'log');
-
-usage(0) if $_help;
-version() if $_version;
-usage(1) unless defined $cmd;
-load_authors() if $_authors;
-
 # make sure we're always running
 unless ($cmd =~ /(?:clone|init|multi-init)$/) {
 	unless (-d $ENV{GIT_DIR}) {
@@ -231,6 +214,24 @@ unless ($cmd =~ /(?:clone|init|multi-init)$/) {
 		$ENV{GIT_DIR} = $git_dir;
 	}
 }
+
+my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
+
+read_repo_config(\%opts);
+Getopt::Long::Configure('pass_through') if ($cmd && $cmd eq 'log');
+my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
+                    'minimize-connections' => \$Git::SVN::Migration::_minimize,
+                    'id|i=s' => \$Git::SVN::default_ref_id,
+                    'svn-remote|remote|R=s' => sub {
+                       $Git::SVN::no_reuse_existing = 1;
+                       $Git::SVN::default_repo_id = $_[1] });
+exit 1 if (!$rv && $cmd && $cmd ne 'log');
+
+usage(0) if $_help;
+version() if $_version;
+usage(1) unless defined $cmd;
+load_authors() if $_authors;
+
 unless ($cmd =~ /^(?:clone|init|multi-init|commit-diff)$/) {
 	Git::SVN::Migration::migration_check();
 }
-- 
1.5.3.6.1992.g3d0f

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Nicolas Pitre @ 2007-11-24 15:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <20071124113814.GA17861@sigill.intra.peff.net>

On Sat, 24 Nov 2007, Jeff King wrote:

> On Fri, Nov 23, 2007 at 01:23:44PM +0000, Johannes Schindelin wrote:
> 
> > Maybe we could change the (non-fast forward) message into (non-fast 
> > forward; need to pull?).
> 
> Not unreasonable, although I think our line length is getting a bit
> long.  Rejected refs would look something like (actually they say
> "[rejected]" but the text is column-aligned with the X's):
> 
>  ! XXXXXXX...XXXXXXX ref_name -> ref_name (non-fast forward; need to pull?)
> 
> There's 58 characters of text not including the two ref_names, leaving
> about 11 characters for each ref name. The name of this topic,
> jk/send-pack, would overflow an 80-character terminal:
> 
>  ! [rejected]        jk/send-pack -> jk/send-pack (non-fast forward; need to pull?)

I personally think this is a bad idea, especially after all the efforts 
that has been put into making those lines not to wrap.

Yet the message itself is not totally accurate either, since "need to 
pull" might have to be "need to force" in some cases.

I think that would be better to append a single line at the end of the 
display with a clue about what "non fast forward" means.


Nicolas

^ permalink raw reply

* Re: [RFC/PATCH] git-help: add new options -w (for web) and -i (for info)
From: Alex Riesen @ 2007-11-24 17:33 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Christian Couder, Junio Hamano, git, Theodore Tso
In-Reply-To: <47482116.70600@op5.se>

Andreas Ericsson, Sat, Nov 24, 2007 14:03:18 +0100:
> Alex Riesen wrote:
>> Christian Couder, Sat, Nov 24, 2007 05:16:50 +0100:
>>> +valid_tool() {
>>> +	case "$1" in
>>> +		firefox | iceweasel | konqueror | w3m)
>>> +			;; # happy
>> lynx, links and dillo?
>
> Why bother validating it at all? I might be using a script to

Kind of my point

^ permalink raw reply

* Re: [RFC/PATCH] git-help: add new options -w (for web) and -i (for info)
From: Matthieu Moy @ 2007-11-24 18:23 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Alex Riesen, Christian Couder, Junio Hamano, git, Theodore Tso
In-Reply-To: <47482116.70600@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> Alex Riesen wrote:
>> Christian Couder, Sat, Nov 24, 2007 05:16:50 +0100:
>>> +valid_tool() {
>>> +	case "$1" in
>>> +		firefox | iceweasel | konqueror | w3m)
>>> +			;; # happy
>>
>> lynx, links and dillo?
>>
>
> Why bother validating it at all? I might be using a script to
> add some formatting and then pass it through the browser of my
> choice. A friend of mine who's visually impaired does just that
> to strip some CSS tags which can make docs quite unreadable for
> him.

In any case, make sure it's consistant with git instaweb, which also
runs a web browser (firefox by default, overridable with --browser or
instaweb.browser).

-- 
Matthieu

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Junio C Hamano @ 2007-11-24 19:09 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, Johannes Schindelin, git, J. Bruce Fields
In-Reply-To: <alpine.LFD.0.99999.0711241042011.9605@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> Yet the message itself is not totally accurate either, since "need to 
> pull" might have to be "need to force" in some cases.

We used to say only "this is not a fast-forward", and did not
mention "what next".  Later we added "maybe you want to pull
first?" without making it clear enough that the reason why the
suggestion may or may not apply to the user is because it
depended largely on the user's workflow.  It probably was a
mistake and not mentioning "what next" at all might have been
less confusion-prone.  I dunno.

> I think that would be better to append a single line at the end of the 
> display with a clue about what "non fast forward" means.

I'd agree, but having said all the above, I am not entirely
happy not mentioning "what next" at all.

There are two equally valid "what next" after your push is
rejected due to non-fast-forwardness.

 (1) You know what you are doing.

   - You are pushing into a "back-up" repository, not for a
     public consumption.

   - You are pushing a branch that are advertised to rebase and
     rewind into your own publishing repository, and other
     people interacting with the branch know about this.

   - You pushed a wrong head there very recently and are fairly
     confident that nobody has seen that mistake, and pushing
     the correct one to fix the mistake.

     In these cases, forcing the push is the right solution
     (except that the third one is dubious, because it depends
     heavily on the "fairly confident" part).

 (2) You were building on a stale head, and were indeed about to
     lose others' changes with a non-fast-forward push.

     The right solution is to rebuild what you push so that you
     will not lose others' changes.  Rebuilding can take two
     different forms:

   - You may want to git-fetch and rebase your work on top of
     others'.

   - You may want to git-pull, which will merge your work with
     what others did.

But of couse the above is way too long as the help text.

Does the user-manual talk about this?  It has a really good
description of how to notice when a merge is not resolved
automatically and what to do next ("Resolving a merge" section).
Perhaps we can enhance "Pushing changes to a public repository"
section to include "what if the push is refused" information.

^ permalink raw reply

* Re: Wishlist for a bundle-only transport mode
From: Junio C Hamano @ 2007-11-24 19:15 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Jakub Narebski, Johannes Schindelin, git
In-Reply-To: <8aa486160711230839o5db5fca1wc09a834ab3d4f053@mail.gmail.com>

"Santi Béjar" <sbejar@gmail.com> writes:

> On Nov 23, 2007 5:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> Maybe I am missing something from the discussion, but what
>> information loss are you referring to?
>
> Because you create an incremental bundle, so all the objects in the
> old bundle will
> not be in the new bundle. But it can be considered the natural
> behavior of bundles.

It might be natural if you are thinking within the limit of "git
bundle create", but it is not natural for "git push" at all.

I think treating a bundle as if it is a bare repository with a
funny representation wouldn't be so wrong, at least from the
user interface point of view.  IOW, I think it is natural for
these:

	$ git push $remote $refspec
        $ git fetch $remote $refspec

to work as "expected" when $remote is actually a local file that
is a bundle, and in fact, "git fetch" should already work that
way.

What's "expected" for a push?  You push from your repository the
objects needed to complete LHS of given $refspec into the
$remote, and then update the refs in the $remote specified by
the $refspec.  There is no deletion of existing objects from the
$remote.  That is what's expected for a push.

So if you want to implement "pushing into a bundle", the
implementation would be:

  * Find the required objects in the existing bundle.  If the
    bundle file does not exist, it might be natural to treat it
    as if you are pushing into an empty but initialized regular
    repository.  If we choose to do this, for a nonexistent
    bundle file, the set of required objects is an empty set.

  * Find the recorded heads in the existing bundle.  Add or
    replace them with the RHS of $refspecs being pushed to come
    up with the new set of heads for the updated bundle.  We
    would want to perform the ordinary "fast-forward" safety and
    reject a push as needed.

  * If there are HEADs in the updated bundle that the pushing
    repository does not have, fetch them (and their required
    objects) first, as it is necessary for the next step.

  * Run this pipeline in the pushing repository to generate a
    packdata stream:

    $ git rev-list --objects <heads in the updated bundle> \
    	--not <required objects in the bundle> |
      git pack-objects --stdout

    This packdata stream will be the payload of the updated
    bundle.

  * The updated bundle will require the same set of objects as
    the bundle before the update.

This is quite different from the way how the other "transports"
are implemented internally to push into usual repositories, but
that is perfectly fine.  What the end user sees will be
consistent if you implement "push into bundle" that way and that
is what matters.

	Note.  I am not saying that we _should_ allow pushing
	into a bundle to update.  I am just saying that if we
	were to implement "git push" into a bundle, that should
        behave as close as other push transports from the end
	uesr's point of view.

If you want different "object losing" semantics, "git bundle
create" to create a new bundle is already there for you.  You
just shouldn't overload that different semantics to "git push",
because that would confuse users without much gain.

^ permalink raw reply

* Re: [PATCH 2/3] Move pathspec validation into interactive_add
From: Junio C Hamano @ 2007-11-24 19:15 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git, peff
In-Reply-To: <1195908922-88023-3-git-send-email-win@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:


> Instead of throwing away the return status of pathspec_match() I am
> keeping it, and any successful match breaks out of the loop early.

Leaving it early before checking if all the given pathspecs are
used defeats the whole "error-unmatch" business, doesn't it?

> Another issue is that simple shell shortcuts don't work, so something
> as simple as "git-add -i ." will report:
>
>   error: pathspec '.' did not match any file(s) known to git.

The sample code snippet I sent you probably is not doing 
get_pathspec() before using the "pattern" thing.  And I suspect
that ...

> Likewise it's not possible to validate pathspecs like "\*.sh" either,

... may be related to that.

^ permalink raw reply

* [PATCH 3/2] core.whitespace: documentation updates.
From: Junio C Hamano @ 2007-11-24 20:09 UTC (permalink / raw)
  To: git
In-Reply-To: <7v4pfcs20b.fsf@gitster.siamese.dyndns.org>

This adds description of core.whitespace to the manual page of git-config,
and updates the stale description of whitespace handling in the manual
page of git-apply.

Also demote "strip" to a synonym status for "fix" as the value of --whitespace
option given to git-apply.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is meant to conclude the "War on more than 8-SP indent" Bruce
   started some time ago.  It is now configurable, and turned off by
   default, so hopefully people outside the kernel circle would not mind.

   A possible addition to the repertoire of core.whitespace is to add
   "cr-at-end", which would consider a line that ends with CR an error.
   We redefine "trailing-space" not to complain to a line ending with
   CRLF but otherwise does not have trailing whitespaces.  To be
   compatible with the current behaviour, cr-at-end needs to be added to
   the default set of errors to be detected, but it might be an
   improvement if we stopped treating 'cr-at-end' as an error by
   default.

 Documentation/config.txt    |   18 ++++++++++++++++--
 Documentation/git-apply.txt |   35 +++++++++++++++++++++--------------
 builtin-apply.c             |    2 +-
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index edf50cd..0e71137 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -293,6 +293,20 @@ core.pager::
 	The command that git will use to paginate output.  Can be overridden
 	with the `GIT_PAGER` environment variable.
 
+core.whitespace::
+	A comma separated list of common whitespace problems to
+	notice.  `git diff` will use `color.diff.whitespace` to
+	highlight them, and `git apply --whitespace=error` will
+	consider them as errors:
++
+* `trailing-space` treats trailing whitespaces at the end of the line
+  as an error (enabled by default).
+* `space-before-tab` treats a space character that appears immediately
+  before a tab character in the initial indent part of the line as an
+  error (enabled by default).
+* `indent-with-non-tab` treats a line that is indented with 8 or more
+  space characters that can be replaced with tab characters.
+
 alias.*::
 	Command aliases for the gitlink:git[1] command wrapper - e.g.
 	after defining "alias.last = cat-file commit HEAD", the invocation
@@ -378,8 +392,8 @@ color.diff.<slot>::
 	which part of the patch to use the specified color, and is one
 	of `plain` (context text), `meta` (metainformation), `frag`
 	(hunk header), `old` (removed lines), `new` (added lines),
-	`commit` (commit headers), or `whitespace` (highlighting dubious
-	whitespace).  The values of these variables may be specified as
+	`commit` (commit headers), or `whitespace` (highlighting
+	whitespace errors). The values of these variables may be specified as
 	in color.branch.<slot>.
 
 color.pager::
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index c1c54bf..bae3e7b 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	  [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
 	  [--allow-binary-replacement | --binary] [--reject] [-z]
 	  [-pNUM] [-CNUM] [--inaccurate-eof] [--cached]
-	  [--whitespace=<nowarn|warn|error|error-all|strip>]
+	  [--whitespace=<nowarn|warn|fix|error|error-all>]
 	  [--exclude=PATH] [--verbose] [<patch>...]
 
 DESCRIPTION
@@ -135,25 +135,32 @@ discouraged.
 	be useful when importing patchsets, where you want to exclude certain
 	files or directories.
 
---whitespace=<option>::
-	When applying a patch, detect a new or modified line
-	that ends with trailing whitespaces (this includes a
-	line that solely consists of whitespaces).  By default,
-	the command outputs warning messages and applies the
-	patch.
-	When gitlink:git-apply[1] is used for statistics and not applying a
-	patch, it defaults to `nowarn`.
-	You can use different `<option>` to control this
-	behavior:
+--whitespace=<action>::
+	When applying a patch, detect a new or modified line that has
+	whitespace errors.  What are considered whitespace errors is
+	controlled by `core.whitespace` configuration.  By default,
+	trailing whitespaces (including lines that solely consist of
+	whitespaces) and a space character that is immediately followed
+	by a tab character inside the initial indent of the line are
+	considered whitespace errors.
++
+By default, the command outputs warning messages but applies the patch.
+When gitlink:git-apply[1] is used for statistics and not applying a
+patch, it defaults to `nowarn`.
++
+You can use different `<action>` to control this
+behavior:
 +
 * `nowarn` turns off the trailing whitespace warning.
 * `warn` outputs warnings for a few such errors, but applies the
-  patch (default).
+  patch as-is (default).
+* `fix` outputs warnings for a few such errors, and applies the
+  patch after fixing them (`strip` is a synonym --- the tool
+  used to consider only trailing whitespaces as errors, and the
+  fix involved 'stripping' them, but modern gits do more).
 * `error` outputs warnings for a few such errors, and refuses
   to apply the patch.
 * `error-all` is similar to `error` but shows all errors.
-* `strip` outputs warnings for a few such errors, strips out the
-  trailing whitespaces and applies the patch.
 
 --inaccurate-eof::
 	Under certain circumstances, some versions of diff do not correctly
diff --git a/builtin-apply.c b/builtin-apply.c
index e04b493..57efcd5 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -45,7 +45,7 @@ static const char *fake_ancestor;
 static int line_termination = '\n';
 static unsigned long p_context = ULONG_MAX;
 static const char apply_usage[] =
-"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|error|error-all|strip>] <patch>...";
+"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
 
 static enum ws_error_action {
 	nowarn_ws_error,
-- 
1.5.3.6.1991.ge56ac

^ permalink raw reply related

* Re: [PATCH 3/2] core.whitespace: documentation updates.
From: J. Bruce Fields @ 2007-11-24 20:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7ik7quc6.fsf_-_@gitster.siamese.dyndns.org>

On Sat, Nov 24, 2007 at 12:09:13PM -0800, Junio C Hamano wrote:
> This adds description of core.whitespace to the manual page of git-config,
> and updates the stale description of whitespace handling in the manual
> page of git-apply.
> 
> Also demote "strip" to a synonym status for "fix" as the value of --whitespace
> option given to git-apply.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * This is meant to conclude the "War on more than 8-SP indent" Bruce
>    started some time ago.  It is now configurable, and turned off by
>    default, so hopefully people outside the kernel circle would not mind.
> 
>    A possible addition to the repertoire of core.whitespace is to add
>    "cr-at-end", which would consider a line that ends with CR an error.
>    We redefine "trailing-space" not to complain to a line ending with
>    CRLF but otherwise does not have trailing whitespaces.  To be
>    compatible with the current behaviour, cr-at-end needs to be added to
>    the default set of errors to be detected, but it might be an
>    improvement if we stopped treating 'cr-at-end' as an error by
>    default.

I'd still prefer this to be a gitattributes thing rather than a config
variable[1].  Last time I raised this you said something to the effect
of "I think you're right, let's fix that before it's merged."  Would you
like me to work on that?

--b.

[1] A rehash of the argument: config variables vary depending on the
system (/etc/gitconfig), the user (~/.config), or the particular
repository ($GIT_DIR/config).  But you don't want the whitespace policy
to vary that way: all users and repositories should see the same
whitespace policy for a given project.  It's entirely possible, however,
that you might like the policy to vary depending on the file (Makefile
vs. main.py?).  And of course getting the policy versioned and
distributed to other repositories automatically is nice too.

> 
>  Documentation/config.txt    |   18 ++++++++++++++++--
>  Documentation/git-apply.txt |   35 +++++++++++++++++++++--------------
>  builtin-apply.c             |    2 +-
>  3 files changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index edf50cd..0e71137 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -293,6 +293,20 @@ core.pager::
>  	The command that git will use to paginate output.  Can be overridden
>  	with the `GIT_PAGER` environment variable.
>  
> +core.whitespace::
> +	A comma separated list of common whitespace problems to
> +	notice.  `git diff` will use `color.diff.whitespace` to
> +	highlight them, and `git apply --whitespace=error` will
> +	consider them as errors:
> ++
> +* `trailing-space` treats trailing whitespaces at the end of the line
> +  as an error (enabled by default).
> +* `space-before-tab` treats a space character that appears immediately
> +  before a tab character in the initial indent part of the line as an
> +  error (enabled by default).
> +* `indent-with-non-tab` treats a line that is indented with 8 or more
> +  space characters that can be replaced with tab characters.
> +
>  alias.*::
>  	Command aliases for the gitlink:git[1] command wrapper - e.g.
>  	after defining "alias.last = cat-file commit HEAD", the invocation
> @@ -378,8 +392,8 @@ color.diff.<slot>::
>  	which part of the patch to use the specified color, and is one
>  	of `plain` (context text), `meta` (metainformation), `frag`
>  	(hunk header), `old` (removed lines), `new` (added lines),
> -	`commit` (commit headers), or `whitespace` (highlighting dubious
> -	whitespace).  The values of these variables may be specified as
> +	`commit` (commit headers), or `whitespace` (highlighting
> +	whitespace errors). The values of these variables may be specified as
>  	in color.branch.<slot>.
>  
>  color.pager::
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index c1c54bf..bae3e7b 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
>  	  [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
>  	  [--allow-binary-replacement | --binary] [--reject] [-z]
>  	  [-pNUM] [-CNUM] [--inaccurate-eof] [--cached]
> -	  [--whitespace=<nowarn|warn|error|error-all|strip>]
> +	  [--whitespace=<nowarn|warn|fix|error|error-all>]
>  	  [--exclude=PATH] [--verbose] [<patch>...]
>  
>  DESCRIPTION
> @@ -135,25 +135,32 @@ discouraged.
>  	be useful when importing patchsets, where you want to exclude certain
>  	files or directories.
>  
> ---whitespace=<option>::
> -	When applying a patch, detect a new or modified line
> -	that ends with trailing whitespaces (this includes a
> -	line that solely consists of whitespaces).  By default,
> -	the command outputs warning messages and applies the
> -	patch.
> -	When gitlink:git-apply[1] is used for statistics and not applying a
> -	patch, it defaults to `nowarn`.
> -	You can use different `<option>` to control this
> -	behavior:
> +--whitespace=<action>::
> +	When applying a patch, detect a new or modified line that has
> +	whitespace errors.  What are considered whitespace errors is
> +	controlled by `core.whitespace` configuration.  By default,
> +	trailing whitespaces (including lines that solely consist of
> +	whitespaces) and a space character that is immediately followed
> +	by a tab character inside the initial indent of the line are
> +	considered whitespace errors.
> ++
> +By default, the command outputs warning messages but applies the patch.
> +When gitlink:git-apply[1] is used for statistics and not applying a
> +patch, it defaults to `nowarn`.
> ++
> +You can use different `<action>` to control this
> +behavior:
>  +
>  * `nowarn` turns off the trailing whitespace warning.
>  * `warn` outputs warnings for a few such errors, but applies the
> -  patch (default).
> +  patch as-is (default).
> +* `fix` outputs warnings for a few such errors, and applies the
> +  patch after fixing them (`strip` is a synonym --- the tool
> +  used to consider only trailing whitespaces as errors, and the
> +  fix involved 'stripping' them, but modern gits do more).
>  * `error` outputs warnings for a few such errors, and refuses
>    to apply the patch.
>  * `error-all` is similar to `error` but shows all errors.
> -* `strip` outputs warnings for a few such errors, strips out the
> -  trailing whitespaces and applies the patch.
>  
>  --inaccurate-eof::
>  	Under certain circumstances, some versions of diff do not correctly
> diff --git a/builtin-apply.c b/builtin-apply.c
> index e04b493..57efcd5 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -45,7 +45,7 @@ static const char *fake_ancestor;
>  static int line_termination = '\n';
>  static unsigned long p_context = ULONG_MAX;
>  static const char apply_usage[] =
> -"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|error|error-all|strip>] <patch>...";
> +"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
>  
>  static enum ws_error_action {
>  	nowarn_ws_error,
> -- 
> 1.5.3.6.1991.ge56ac
> 
> -
> 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: [PATCH 3/2] core.whitespace: documentation updates.
From: Junio C Hamano @ 2007-11-24 21:42 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: git
In-Reply-To: <20071124202257.GC12864@fieldses.org>

"J. Bruce Fields" <bfields@fieldses.org> writes:

> I'd still prefer this to be a gitattributes thing rather than a config
> variable[1].  Last time I raised this you said something to the effect
> of "I think you're right, let's fix that before it's merged."  Would you
> like me to work on that?

Ah, I forgot about that, and you are right.  Go wild.

^ permalink raw reply

* Re: [PATCH 2/3] Move pathspec validation into interactive_add
From: Wincent Colaiuta @ 2007-11-24 21:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff
In-Reply-To: <7vir3rqwts.fsf@gitster.siamese.dyndns.org>

El 24/11/2007, a las 20:15, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> Instead of throwing away the return status of pathspec_match() I am
>> keeping it, and any successful match breaks out of the loop early.
>
> Leaving it early before checking if all the given pathspecs are
> used defeats the whole "error-unmatch" business, doesn't it?

No, I probably didn't explain that clearly enough... If you look at  
the patch, I break out of the *inner* loop the first time a particular  
pattern matches. Then I move on to the next pattern, and any single  
invalid pattern will be enough to provide the "error-unmatch"  
indication.

>> Another issue is that simple shell shortcuts don't work, so something
>> as simple as "git-add -i ." will report:
>>
>>  error: pathspec '.' did not match any file(s) known to git.
>
> The sample code snippet I sent you probably is not doing
> get_pathspec() before using the "pattern" thing.  And I suspect
> that ...
>
>> Likewise it's not possible to validate pathspecs like "\*.sh" either,
>
> ... may be related to that.


Ok, thanks for the pointer. Didn't know what the get_pathspec()  
function was for, but will have a play with it.

Cheers,
Wincent

^ permalink raw reply

* Re: [PATCH 2/3] Move pathspec validation into interactive_add
From: Junio C Hamano @ 2007-11-24 22:34 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git, peff
In-Reply-To: <4D81F973-8951-458A-958D-E24C826CA548@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:

> El 24/11/2007, a las 20:15, Junio C Hamano escribió:
>
>> Wincent Colaiuta <win@wincent.com> writes:
>>
>>> Instead of throwing away the return status of pathspec_match() I am
>>> keeping it, and any successful match breaks out of the loop early.
>>
>> Leaving it early before checking if all the given pathspecs are
>> used defeats the whole "error-unmatch" business, doesn't it?
>
> No, I probably didn't explain that clearly enough... If you look at
> the patch, I break out of the *inner* loop the first time a particular
> pattern matches. Then I move on to the next pattern, and any single
> invalid pattern will be enough to provide the "error-unmatch"
> indication.

Heh, I missed that the code was doing something so stupid ;-).

The helper function pathspec_match() takes a single path and a
set of pathspecs (NULL terminated), and says if the path is
covered with that set, while recording which one of the
pathspecs caused the match in the ps_matched array.

You are checking first with the full set of patterns, then the
full set minus the first pattern, and then the full set minus
the first two patterns, and so on.  No wonder it is inefficient.

Why are you trying to micro-optimize this part in the first
place?  Have you benchmarked and determined that iterating over
full cache is the bottleneck?

In order to prove that there is a pathspec (or more) that does
not match anything in the cache, you need to interate over the
whole cache at least once.  The only case you can short-cut is
when you can tell that all of them have been used before you
iterated over all cache entries.

So lose that silly outer loop, and replace the inner function
with something like this:

        static int validate_pathspec(const char *prefix, const char **pattern)
        {
                int i, ret = 0, num_patterns;
                char *m;

                if (!pattern || !*pattern)
                        return 0;

                for (num_patterns = 0; pattern[num_patterns]; num_patterns++)
                        ;
                m = xcalloc(1, num_patterns + 1);
                for (i = 0; i < active_nr; i++) {
                        struct cache_entry *ce = active_cache[i];

                        if (pathspec_match(pattern, m, ce->name, 0)) {
                                /*
                                 * You could micro optimize by leaving
                                 * the loop early when you notice that all
				 * patterns are used.
				 *
                                 * if (strlen(m) == num_patterns)
                                 * 	goto ok;
				 *
                                 */ 
				; /* nothing */
			}
                }
                report_path_error(m, pattern, prefix ? strlen(prefix) : 0);
        ok:
                free(m);
                return ret;
        }

My gut feeling is that the micro-optimization is not worth it here, but
I didn't try.  I think without the micro-optimization the above is the
same as I gave you earlier.

^ permalink raw reply

* [RFC/PATCH (homepage)] Add links to binaries for MS Windows (Cygwin, msysGit)
From: Jakub Narebski @ 2007-11-24 22:47 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Jakub Narebski

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 index.html |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/index.html b/index.html
index 29fa0b1..142bd46 100644
--- a/index.html
+++ b/index.html
@@ -236,6 +236,16 @@ You can also use one of many <a href="http://www.kernel.org/mirrors/">kernel.org
 <td><a href="http://packages.debian.org/unstable/devel/git-core">http://packages.debian.org/unstable/devel/git-core</a></td>
 </tr>
 
+<tr>
+<td rowspan="2">Win</td>
+<td><a href="http://www.cygwin.com/">Cygwin</a></td>
+<td><a href="http://www.cygwin.com/setup.exe">http://www.cygwin.com/setup.exe</a></td>
+</tr>
+<tr class="odd">
+<td><a href="http://code.google.com/p/msysgit/">msysGit</a></td>
+<td><a href="http://code.google.com/p/msysgit/downloads/list">http://code.google.com/p/msysgit/downloads/list</a></td>
+</tr>
+
 </table>
 
 <h3>Development snapshots</h3>
-- 
1.5.3.6

^ permalink raw reply related

* Backing git repos to tape?
From: Joakim Tjernlund @ 2007-11-25 10:58 UTC (permalink / raw)
  To: git

Is there a preferred way to do the above?

To do a full backup I could just copy the whole repo, but how
do I do inrecmental backups(and restore)?

 Jocke 

^ permalink raw reply

* Re: Backing git repos to tape?
From: Jakub Narebski @ 2007-11-25 11:05 UTC (permalink / raw)
  To: git
In-Reply-To: <027601c82f52$2433fdf0$5267a8c0@Jocke>

[Cc: Joakim Tjernlund <joakim.tjernlund@transmode.se>, git@vger.kernel.org]

Joakim Tjernlund wrote:

> Is there a preferred way to do the above?
> 
> To do a full backup I could just copy the whole repo, but how
> do I do inrecmental backups(and restore)?

Try git-bundle. There were in the mailing list archive the recipe
on how to do an "incremental" bundle...

...ahh, there it is, by Johannes "Dscho" Schindelin:

        git bundle create retort.bundle --all \
                --not $(git ls-remote the-other.bundle | cut -c1-40)

Message-ID: <Pine.LNX.4.64.0711231132120.27959@racer.site>

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Backing git repos to tape?
From: Jakub Narebski @ 2007-11-25 12:00 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: git
In-Reply-To: <028101c82f58$0e26f200$5267a8c0@Jocke>

On Sun, 25 Nov 2007, Joakim Tjernlund wrote:
> Joakim Tjernlund wrote: 
>> Jakub Narebski wrote:
>>> Joakim Tjernlund wrote:
>>> 
>>>> Is there a preferred way to do the above?
>>>> 
>>>> To do a full backup I could just copy the whole repo, but how
>>>> do I do inrecmental backups(and restore)?
>>> 
>>> Try git-bundle. There were in the mailing list archive the recipe
>>> on how to do an "incremental" bundle...
>>> 
>>> ..ahh, there it is, by Johannes "Dscho" Schindelin:
>>> 
>>>         git bundle create retort.bundle --all \
>>>                 --not $(git ls-remote the-other.bundle | cut -c1-40)
>> 
>> Thanks, will look into git bundle

Please note however thatgit-bundle was created for easy and efficient
off-line (sneakernet) transport, not for backups of repositories.

Nevertheless it might be a good tool to use.
 
> Just did a
>   git ls-remote ./.
> in my linux repo and got a lot of ^{} in the printout.
> What do the ^{} mean?
>  
> ...
> 0b8bc8b91cf6befea20fe78b90367ca7b61cfa0d        refs/tags/v2.6.23
> 7d57c74238cdf570bca20b711b2c0b31a553c1e5        refs/tags/v2.6.23-rc1
> f695baf2df9e0413d3521661070103711545207a        refs/tags/v2.6.23-rc1^{}
[...]

----
git-rev-parse(1):

SPECIFYING REVISIONS
--------------------

A revision parameter typically, but not necessarily, names a
commit object.  They use what is called an 'extended SHA1'
syntax.  Here are various ways to spell object names.

[...]

* A suffix '^' followed by an empty brace pair
  (e.g. `v0.99.8^{}`) means the object could be a tag,
  and dereference the tag recursively until a non-tag object is
  found.

----

refs/tags/v2.6.23-rc1 is a ref which points to a _tag object_ 7d57c742...,
which in turn points to commit f695baf2...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH 0/3] Updates to git-add--interactive
From: Wincent Colaiuta @ 2007-11-25 13:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff

Here's an updated series, this time hopefully with the braindead
parts removed.

[PATCH 1/3] Add "--patch" option to git-add--interactive

    Unchanged.

[PATCH 2/3] Move pathspec validation into interactive_add

    Braindeadness removed.

[PATCH 3/3] Rename patch_update_file function to patch_update_pathspec

    Only modified insofar as to make it apply on top of PATCH 2.

The one remaining issue is that pathspecs like "\*.h" don't work.
But from comments earlier on in this thread I had the impression
that they should.

They don't work because such pathspecs don't work for the underlying
git-diff-files implementation that's used by git-add--interactive:

git diff-files --numstat --summary -- \*.h

Is some alternative being proposed that would allow them to work?

^ permalink raw reply

* [PATCH 1/3] Rename patch_update_file function to patch_update_pathspec
From: Wincent Colaiuta @ 2007-11-25 13:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Wincent Colaiuta
In-Reply-To: <1195996542-86074-1-git-send-email-win@wincent.com>

The patch_update_file function really works on pathspecs, not files, so
rename it to reflect its actual purpose.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-add--interactive.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index e347216..15b2c9f 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -571,11 +571,11 @@ sub patch_update_cmd {
 				       HEADER => $status_head, },
 				     @mods);
 	for (@them) {
-		patch_update_file($_->{VALUE});
+		patch_update_pathspec($_->{VALUE});
 	}
 }
 
-sub patch_update_file {
+sub patch_update_pathspec {
 	my ($ix, $num);
 	my $path = shift;
 	my ($head, @hunk) = parse_diff($path);
-- 
1.5.3.6.1994.g38001

^ permalink raw reply related

* [PATCH 2/3] Move pathspec validation into interactive_add
From: Wincent Colaiuta @ 2007-11-25 13:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Wincent Colaiuta
In-Reply-To: <1195996542-86074-2-git-send-email-win@wincent.com>

Simplify git-add--interactive by moving the pathspec validation into the
interactive_add() function of builtin-add. We can do this because
builtin-add is the only caller of git-add--interactive.

The validate_pathspec() function added by this commit is based on a
sample posted to the mailing list by Junio Hamano.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 builtin-add.c             |   33 +++++++++++++++++++++++++++++----
 builtin-commit.c          |    2 +-
 commit.h                  |    2 +-
 git-add--interactive.perl |   12 ++----------
 4 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index dd895df..870f4a1 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -135,12 +135,37 @@ static void refresh(int verbose, const char **pathspec)
         free(seen);
 }
 
-int interactive_add(int argc, const char **argv)
+static int validate_pathspec(const char *prefix, const char **patterns)
+{
+	int i, ret = 0;
+	char *m;
+	if (!patterns || !*patterns)
+		return 0;
+	if (read_cache() < 0)
+		die("index file corrupt");
+	(void)get_pathspec(prefix, patterns);
+	for (i = 0; patterns[i]; i++)
+		;
+	m = xcalloc(1, i);
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+		(void)pathspec_match(patterns, m, ce->name, 0);
+	}
+	ret = report_path_error(m, patterns, prefix ? strlen(prefix) : 0);
+	free(m);
+	return ret;
+}
+
+int interactive_add(const char *prefix, int argc, const char **argv)
 {
 	int status;
-	const char **args = xmalloc(sizeof(const char *) * (argc + 1));
+	const char **args;
+	if ((status = validate_pathspec(prefix, argv)))
+		return status;
+	args = xmalloc(sizeof(const char *) * (argc + 2));
 	args[0] = "add--interactive";
-	memcpy((void *)args + sizeof(const char *), argv, sizeof(const char *) * argc);
+	memcpy((void *)args + sizeof(const char *),
+	    argv, sizeof(const char *) * argc);
 	args[argc + 1] = NULL;
 
 	status = run_command_v_opt(args, RUN_GIT_CMD);
@@ -176,7 +201,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
 	if (add_interactive)
-		exit(interactive_add(argc, argv));
+		exit(interactive_add(prefix, argc, argv));
 
 	git_config(git_default_config);
 
diff --git a/builtin-commit.c b/builtin-commit.c
index 5d27102..95d1c0d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -165,7 +165,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 	const char **pathspec = NULL;
 
 	if (interactive) {
-		interactive_add(argc, argv);
+		interactive_add(prefix, argc, argv);
 		commit_style = COMMIT_AS_IS;
 		return get_index_file();
 	}
diff --git a/commit.h b/commit.h
index 9f0765b..dc6fe31 100644
--- a/commit.h
+++ b/commit.h
@@ -113,7 +113,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
 
 int in_merge_bases(struct commit *, struct commit **, int);
 
-extern int interactive_add(int argc, const char **argv);
+extern int interactive_add(const char *prefix, int argc, const char **argv);
 extern int rerere(void);
 
 static inline int single_parent(struct commit *commit)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 15b2c9f..381bcbe 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -56,17 +56,9 @@ sub list_modified {
 	my ($only) = @_;
 	my (%data, @return);
 	my ($add, $del, $adddel, $file);
-	my @tracked = ();
-
-	if (@ARGV) {
-		@tracked = map {
-			chomp $_; $_;
-		} run_cmd_pipe(qw(git ls-files --exclude-standard --), @ARGV);
-		return if (!@tracked);
-	}
 
 	for (run_cmd_pipe(qw(git diff-index --cached
-			     --numstat --summary HEAD --), @tracked)) {
+			     --numstat --summary HEAD --), @ARGV)) {
 		if (($add, $del, $file) =
 		    /^([-\d]+)	([-\d]+)	(.*)/) {
 			my ($change, $bin);
@@ -89,7 +81,7 @@ sub list_modified {
 		}
 	}
 
-	for (run_cmd_pipe(qw(git diff-files --numstat --summary --), @tracked)) {
+	for (run_cmd_pipe(qw(git diff-files --numstat --summary --), @ARGV)) {
 		if (($add, $del, $file) =
 		    /^([-\d]+)	([-\d]+)	(.*)/) {
 			if (!exists $data{$file}) {
-- 
1.5.3.6.1994.g38001

^ permalink raw reply related

* [PATCH 3/3] Add "--patch" option to git-add--interactive
From: Wincent Colaiuta @ 2007-11-25 13:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Wincent Colaiuta
In-Reply-To: <1195996542-86074-3-git-send-email-win@wincent.com>

When the "--patch" option is supplied, the patch_update_file function is
called, once for each supplied pathspec argument, and then we exit.

Seeing as builtin-add is the only caller of git-add--interactive we can
impose a strict requirement on the format of the arguments to avoid
possible ambiguity: an "--" argument must be used whenever any pathspecs
are passed, both with the "--patch" option and without it.

This commit adds an early return mechanism to the patch_update_pathspec
function to prevent spurious line feeds from being echoed when the user
passes in pathspecs which match unchanged files.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 Documentation/git-add.txt |    9 ++++++++-
 builtin-add.c             |   17 ++++++++++-------
 git-add--interactive.perl |   35 ++++++++++++++++++++++++++++++++---
 3 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 63829d9..ce22de8 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -61,7 +61,14 @@ OPTIONS
 
 -i, \--interactive::
 	Add modified contents in the working tree interactively to
-	the index.
+	the index. Optional path arguments may be supplied to limit
+	operation to a subset of the working tree. See ``Interactive
+	mode'' for details.
+
+-p, \--patch:
+	Similar to Interactive mode but the initial command loop is
+	bypassed and the 'patch' subcommand is invoked using each of
+	the specified filepatterns before exiting.
 
 -u::
 	Update only files that git already knows about. This is similar
diff --git a/builtin-add.c b/builtin-add.c
index 870f4a1..2172e7e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,7 +19,7 @@ static const char * const builtin_add_usage[] = {
 	"git-add [options] [--] <filepattern>...",
 	NULL
 };
-
+static int patch_interactive = 0, add_interactive = 0;
 static int take_worktree_changes;
 
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
@@ -158,15 +158,18 @@ static int validate_pathspec(const char *prefix, const char **patterns)
 
 int interactive_add(const char *prefix, int argc, const char **argv)
 {
-	int status;
+	int status, pre_argc;
 	const char **args;
 	if ((status = validate_pathspec(prefix, argv)))
 		return status;
-	args = xmalloc(sizeof(const char *) * (argc + 2));
+	pre_argc = patch_interactive ? 3 : 2;
+	args = xmalloc(sizeof(const char *) * (argc + pre_argc + 1));
 	args[0] = "add--interactive";
-	memcpy((void *)args + sizeof(const char *),
+	args[1]	= patch_interactive ? "--patch" : "--";
+	args[2] = "--";
+	memcpy((void *)args + sizeof(const char *) * pre_argc,
 	    argv, sizeof(const char *) * argc);
-	args[argc + 1] = NULL;
+	args[argc + pre_argc] = NULL;
 
 	status = run_command_v_opt(args, RUN_GIT_CMD);
 	free(args);
@@ -179,13 +182,13 @@ static const char ignore_error[] =
 "The following paths are ignored by one of your .gitignore files:\n";
 
 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
-static int add_interactive = 0;
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only),
 	OPT__VERBOSE(&verbose),
 	OPT_GROUP(""),
 	OPT_BOOLEAN('i', "interactive", &add_interactive, "interactive picking"),
+	OPT_BOOLEAN('p', "patch", &patch_interactive, "interactive patching"),
 	OPT_BOOLEAN('f', NULL, &ignored_too, "allow adding otherwise ignored files"),
 	OPT_BOOLEAN('u', NULL, &take_worktree_changes, "update tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
@@ -200,7 +203,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
-	if (add_interactive)
+	if (add_interactive || patch_interactive)
 		exit(interactive_add(prefix, argc, argv));
 
 	git_config(git_default_config);
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 381bcbe..7d62ceb 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -2,6 +2,9 @@
 
 use strict;
 
+# command line options
+my $patch;
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32') {
 		my @invalid = grep {m/[":*]/} @_;
@@ -335,7 +338,8 @@ sub add_untracked_cmd {
 
 sub parse_diff {
 	my ($path) = @_;
-	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path)
+	    or return undef;
 	my (@hunk) = { TEXT => [] };
 
 	for (@diff) {
@@ -571,6 +575,7 @@ sub patch_update_pathspec {
 	my ($ix, $num);
 	my $path = shift;
 	my ($head, @hunk) = parse_diff($path);
+	return unless $head;
 	for (@{$head->{TEXT}}) {
 		print;
 	}
@@ -775,6 +780,20 @@ add untracked - add contents of untracked files to the staged set of changes
 EOF
 }
 
+sub process_args {
+	return unless @ARGV;
+	my $arg = shift @ARGV;
+	if ($arg eq "--patch") {
+		$patch = 1;
+		$arg = shift @ARGV or die "missing --";
+		die "invalid argument $arg, expecting --"
+		    unless $arg eq "--";
+	}
+	elsif ($arg ne "--") {
+		die "invalid argument $arg, expecting --";
+	}
+}
+
 sub main_loop {
 	my @cmd = ([ 'status', \&status_cmd, ],
 		   [ 'update', \&update_cmd, ],
@@ -803,6 +822,16 @@ sub main_loop {
 	}
 }
 
+process_args();
 refresh();
-status_cmd();
-main_loop();
+if ($patch) {
+	print "No filepattern specified: what did you want to patch?\n"
+	    unless @ARGV;
+	foreach my $pathspec (@ARGV) {
+		patch_update_pathspec($pathspec);
+	}
+}
+else {
+	status_cmd();
+	main_loop();
+}
-- 
1.5.3.6.1994.g38001

^ permalink raw reply related

* Re: Git Screencast ?
From: Karl Hasselström @ 2007-11-25 16:57 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Gilger, Michael Donaghy, git
In-Reply-To: <20071123205825.GB14735@spearce.org>

On 2007-11-23 15:58:25 -0500, Shawn O. Pearce wrote:

> Actually some users have found that its easier to grok the index by
> using git-gui. You can more directly see the state of update
> actions, and it is fairly easy to stage/unstage individual hunks of
> files through git-gui. Once you get that down then seeking the same
> methods through the command line isn't too difficult,

I spent a good chunk of time getting familiar with the index before
there was a git-gui, and I can only agree: git-gui makes it obvious
how simple the concept of an index really is, in a way the
command-line tools never did. With the command line, you have to
visualize it yourself, which means you have to understand it first;
but with git-gui, it's just drawn on your screen, plain and simple.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [WIP PATCH] Add 'git fast-export', the sister of 'git fast-import'
From: Karl Hasselström @ 2007-11-25 17:00 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Schindelin, hanwen, git
In-Reply-To: <20071123205958.GC14735@spearce.org>

On 2007-11-23 15:59:58 -0500, Shawn O. Pearce wrote:

> I have been considering creating a "git-gui daemon" process that
> links to libgit.a and can be driven bidirectionally through its
> stdin/stdout. Based on git-fast-export, sorta. But I haven't even
> started it...
>
> But the idea is sort of what Han-Wen wants. Why should I fork
> rev-parse to get a ref value? Or update-ref to change one?

Obviously, something like this would be very valuable for StGit as
well.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: Git Screencast ?
From: Johannes Gilger @ 2007-11-25 17:43 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Michael Donaghy, git
In-Reply-To: <20071123205825.GB14735@spearce.org>

On 23/11/07 15:58, Shawn O. Pearce wrote:
> Actually some users have found that its easier to grok the index
> by using git-gui.  You can more directly see the state of update
> actions, and it is fairly easy to stage/unstage individual hunks
> of files through git-gui.  Once you get that down then seeking
> the same methods through the command line isn't too difficult,
> many of the terms in git-gui are the same as the command line term,
> to make the transition back and forth easier for the user.

It seems that I myself did not realize what a good tool git-gui is. I 
started using git with the command line and so far have rarely needed to 
stage individual hunks (or explain the git principles to anyone). I did 
use gitk to visualize history (especially branches) though.

So, to sum it up, yeah, git-gui is a good way to find your way into git. 
After some time you realize yourself when to use the gui and when to 
stick to the command line.

Greetings,
Jojo

-- 
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

^ permalink raw reply

* [RFC] Making ce_path_match() more useful by accepting globs
From: Junio C Hamano @ 2007-11-25 18:03 UTC (permalink / raw)
  To: Linus, "Torvalds <torvalds"; +Cc: git

Currently ce_path_match() only uses "the leading directory" match, and
does not understand file globs.  These do not work:

   git diff-files 't/*.sh'
   git diff-index HEAD 'xdiff/*.c'
   git update-index -g 'Documentation/howto/*.txt'

This teaches the ce_path_match(), the underlying function that are used
for checking if a given cache entry matches the given set of pathspecs,
to use the match_pathspec() from git-ls-files, which knows about glob
patterns.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Having two different behaviours of pathspec matching has been
   bothering me for quite some time.  The changes here look trivially
   correct and the result passes all the tests, but this is quite close
   to the core part of the system, and would benefit greatly from extra
   set of eyes.

   This patch does not touch the tree walker, and does not affect
   diff-tree nor ancestry pruning done in the revision traversal.  That
   however is even closer to the core and is performance critical.  It
   needs to be done carefully not to descend into trees that would never
   match needlessly.  IOW, not today.

 dir.c        |    5 +++--
 read-cache.c |   17 ++---------------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/dir.c b/dir.c
index 225fdfb..be640c9 100644
--- a/dir.c
+++ b/dir.c
@@ -98,20 +98,21 @@ int match_pathspec(const char **pathspec, const char *name, int namelen, int pre
 {
 	int retval;
 	const char *match;
+	int want_seen = !!seen;
 
 	name += prefix;
 	namelen -= prefix;
 
 	for (retval = 0; (match = *pathspec++) != NULL; seen++) {
 		int how;
-		if (retval && *seen == MATCHED_EXACTLY)
+		if (retval && seen && want_seen && *seen == MATCHED_EXACTLY)
 			continue;
 		match += prefix;
 		how = match_one(match, name, namelen);
 		if (how) {
 			if (retval < how)
 				retval = how;
-			if (*seen < how)
+			if (want_seen && *seen < how)
 				*seen = how;
 		}
 	}
diff --git a/read-cache.c b/read-cache.c
index 7db5588..767464e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -472,7 +472,7 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)
 
 int ce_path_match(const struct cache_entry *ce, const char **pathspec)
 {
-	const char *match, *name;
+	const char *name;
 	int len;
 
 	if (!pathspec)
@@ -480,20 +480,7 @@ int ce_path_match(const struct cache_entry *ce, const char **pathspec)
 
 	len = ce_namelen(ce);
 	name = ce->name;
-	while ((match = *pathspec++) != NULL) {
-		int matchlen = strlen(match);
-		if (matchlen > len)
-			continue;
-		if (memcmp(name, match, matchlen))
-			continue;
-		if (matchlen && name[matchlen-1] == '/')
-			return 1;
-		if (name[matchlen] == '/' || !name[matchlen])
-			return 1;
-		if (!matchlen)
-			return 1;
-	}
-	return 0;
+	return !!match_pathspec(pathspec, name, len, 0, NULL);
 }
 
 /*

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox