Git development
 help / color / mirror / Atom feed
* Re: Speedlimit at "git clone"
From: Daniel Stenberg @ 2009-12-06 18:13 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091206144322.GB26440@coredump.intra.peff.net>

On Sun, 6 Dec 2009, Jeff King wrote:

>>> how can i limit the download speed at "git clone"?
>>>
>> no one any idea?
>
> Git has no internal support for doing this.

libcurl provides such a feature so if the clone is done over HTTP it would be 
easy to offer, but I figure it would also be weird to only provide it over a 
single particular transfer method...

-- 

  / daniel.haxx.se

^ permalink raw reply

* Re: [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: Matthieu Moy @ 2009-12-06 18:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljhj4wv0.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> y@imag.fr writes:

(sorry for the bad email, my fingers tried to anwser "yes" when
git-send-email asked me to confirm my address)

>> diff --git a/cache.h b/cache.h
>> index 0e69384..5c8cb5f 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)
>>  #define DEFAULT_ABBREV 7
>>  
>>  extern int get_sha1(const char *str, unsigned char *sha1);
>> -extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
>> +static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
>> +{
>> +	return get_sha1_with_mode_1(str, sha1, mode, 0, NULL);
>> +}
>> +extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix);
>
> Do I understand correctly that "fatal" here is the same as "!gently"
> elsewhere in the API?

It seems it is. I renamed it.

>> +	if (errno == ENOENT || errno == ENOTDIR) {
>> +		char *fullname = malloc(strlen(filename)
>> +					     + strlen(prefix) + 1);
>> +		strcpy(fullname, prefix);
>> +		strcat(fullname, filename);
>
> What if malloc fails here (and elsewhere in your patch)?

Should have been an xmalloc. Fixed.

>> +static void diagnose_invalid_index_path(int stage,
>> +					const char *prefix,
>> +					const char *filename)
>> +{
>> +	struct stat st;
>> +
>> +	if (!prefix)
>> +		prefix = "";
>> +
>> +	if (!lstat(filename, &st))
>> +		die("Path '%s' exists on disk, but not in the index.", filename);
>> +	if (errno == ENOENT || errno == ENOTDIR) {
>> +		struct cache_entry *ce;
>> +		int pos;
>> +		int namelen = strlen(filename) + strlen(prefix);
>> +		char *fullname = malloc(namelen + 1);
>> +		strcpy(fullname, prefix);
>> +		strcat(fullname, filename);
>> +		pos = cache_name_pos(fullname, namelen);
>> +		if (pos < 0)
>> +			pos = -pos - 1;
>> +		ce = active_cache[pos];
>> +		if (ce_namelen(ce) == namelen &&
>> +		    !memcmp(ce->name, fullname, namelen))
>> +			die("Path '%s' is in the index, but not '%s'.\n"
>> +			    "Did you mean ':%d:%s'?",
>> +			    fullname, filename,
>> +			    stage, fullname);
                            ^^^^^
This one should have been ce_stage(ce), otherwise, we
suggest :0:t/Makefile as you probably guessed:

> What happens if the user asked for ":2:Makefile" while in directory "t/",
> and there is ":1:t/Makefile" but not ":2:t/Makefile" in the index?
>
> What should happen if the user asked for ":2:t/Makefile" in such a
> case?

I originally didn't handle the case "it's in the index, but not at the
stage you requested" (since most users asking :N:blah will be fluent
enough in Git not to need a special case of error messages). But I
finally implemented this test.

>> @@ -862,9 +934,24 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
>>  	}
>>  	if (*cp == ':') {
>>  		unsigned char tree_sha1[20];
>> -		if (!get_sha1_1(name, cp-name, tree_sha1))
>> -			return get_tree_entry(tree_sha1, cp+1, sha1,
>> -					      mode);
>> +		char *object_name;
>> +		if (fatal) {
>> +			object_name = malloc(cp-name+1);
>
> Where is this freed?

Nowhere. In the present state, it's not serious since all codepath
going through this malloc reach a die() right after. But a free()
doesn't harm, I've added it.

> Instead of doing a leaky allocation, it may make sense to pass the tree
> object name as <const char *, size_t> pair, and print it with "%.*s" in
> the error reporting codepath.  After all, object_name is used only for
> that purpose in diagnose_invalid_sha1_path(), no?

matter of taste, but I prefer doing one "complicated" thing (copying
the string) once, and avoid having to deal with two variables instead
of one later.

>> +test_expect_success 'correct file objects' '
>> +	HASH_file=$(git rev-parse HEAD:file.txt) &&
>> +	git rev-parse HEAD:subdir/file.txt &&
>> +	git rev-parse :index-only.txt &&
>> +	cd subdir &&
>> +	git rev-parse HEAD:file.txt &&
>> +	git rev-parse HEAD:subdir/file2.txt &&
>> +	test $HASH_file = $(git rev-parse HEAD:file.txt) &&
>> +	test $HASH_file = $(git rev-parse :file.txt) &&
>> +	test $HASH_file = $(git rev-parse :0:file.txt) &&
>> +	cd ..
>> +'
>
> Please make it a habit of not doing "cd" without forcing a subprocess
> using ().

Fixed.

>> +	test_must_fail git rev-parse foobar:file.txt 2>&1 |
>> +		grep "Invalid object name '"'"'foobar'"'"'." &&
>
> It always is better to write this in separate steps, because exit status
> of the upstream of pipe is discarded by the shell.

Fixed.

I'll resubmit soon.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: git-blame.el: what is format-spec?
From: David Kågedal @ 2009-12-06 18:43 UTC (permalink / raw)
  To: Sergei Organov; +Cc: Andreas Schwab, git
In-Reply-To: <87einafojx.fsf@osv.gnss.ru>

Sergei Organov <osv@javad.com> writes:

> Then there should be (require 'format-spec) in git-blame.el, right? Due
> to:

Of course. I must have missed that since I already had it loaded.

> Now, I've evaluated (require 'format-spec) in my Emacs 22 (yes, 22, not
> 23), and now git-blame almost works there. The problem I see is that it
> doesn't output anything in the echo area. It color-codes the buffer, it
> does show correct pop-up when mouse is over a region, but it doesn't
> print anything in the echo area when I move cursor through the regions.
> Any idea how to debug/fix this?

Well, it appears I removed the output to the echo area. I didn't think
it worked very well, and the new output format mostly replaces it by
showing the hash.

There are also technical reasons for removing it (it couldn't be
implemented very cleanly).

It would of course be possible to restore the old way, but I think it
would be good to ask ourselves what we really would like to see? Some
ideas:

* A keybinding to show the commit introducing the current line,
  including diff and all.

* A keybinding to show the commit message in the echo areia.

* The old "log --oneline in echo area when point moves in to a new
  region" behavior.

* The old behvaior with a delay.

* Inline display of even more information. For just the current region?

* A constantly updating separate window

-- 
David Kågedal

^ permalink raw reply

* [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: Matthieu Moy @ 2009-12-06 18:45 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpqhbs4dkjr.fsf@bauges.imag.fr>

The previous error message was the same in many situations (unknown
revision or path not in the working tree). We try to help the user as
much as possible to understand the error, especially with the
sha1:filename notation. In this case, we say whether the sha1 or the
filename is problematic, and diagnose the confusion between
relative-to-root and relative-to-$PWD confusion precisely.

The 7 new error messages are tested.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
This should address Junio's comments (BTW, I forgot to say "thanks for
the review" :-) ).


 cache.h                        |    6 ++-
 setup.c                        |   15 +++++-
 sha1_name.c                    |  115 ++++++++++++++++++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   69 ++++++++++++++++++++++++
 4 files changed, 198 insertions(+), 7 deletions(-)
 create mode 100755 t/t1506-rev-parse-diagnosis.sh

diff --git a/cache.h b/cache.h
index 0e69384..563862c 100644
--- a/cache.h
+++ b/cache.h
@@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
-extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
+static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+{
+	return get_sha1_with_mode_1(str, sha1, mode, 1, NULL);
+}
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
diff --git a/setup.c b/setup.c
index f67250b..5792eb7 100644
--- a/setup.c
+++ b/setup.c
@@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)
 	die_errno("failed to stat '%s'", arg);
 }
 
+static void NORETURN die_verify_filename(const char *prefix, const char *arg)
+{
+	unsigned char sha1[20];
+	unsigned mode;
+	/* try a detailed diagnostic ... */
+	get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);
+	/* ... or fall back the most general message. */
+	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
+	    "Use '--' to separate paths from revisions", arg);
+
+}
+
 /*
  * Verify a filename that we got as an argument for a pathspec
  * entry. Note that a filename that begins with "-" never verifies
@@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)
 		die("bad flag '%s' used after filename", arg);
 	if (check_filename(prefix, arg))
 		return;
-	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
-	    "Use '--' to separate paths from revisions", arg);
+	die_verify_filename(prefix, arg);
 }
 
 /*
diff --git a/sha1_name.c b/sha1_name.c
index 44bb62d..1bc09ac 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -804,7 +804,96 @@ int get_sha1(const char *name, unsigned char *sha1)
 	return get_sha1_with_mode(name, sha1, &unused);
 }
 
-int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
+/* Must be called only when object_name:filename doesn't exist. */
+static void diagnose_invalid_sha1_path(const char *prefix,
+				       const char *filename,
+				       const char *tree_sha1,
+				       const char *object_name)
+{
+	struct stat st;
+	unsigned char sha1[20];
+	unsigned mode;
+
+	if (!prefix)
+		prefix = "";
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in '%s'.",
+		    filename, object_name);
+	if (errno == ENOENT || errno == ENOTDIR) {
+		char *fullname = xmalloc(strlen(filename)
+					     + strlen(prefix) + 1);
+		strcpy(fullname, prefix);
+		strcat(fullname, filename);
+
+		if (!get_tree_entry(tree_sha1, fullname,
+				    sha1, &mode)) {
+			die("Path '%s' exists, but not '%s'.\n"
+			    "Did you mean '%s:%s'?",
+			    fullname,
+			    filename,
+			    object_name,
+			    fullname);
+		}
+		die("Path '%s' does not exist in '%s'",
+		    filename, object_name);
+	}
+}
+
+/* Must be called only when :stage:filename doesn't exist. */
+static void diagnose_invalid_index_path(int stage,
+					const char *prefix,
+					const char *filename)
+{
+	struct stat st;
+	struct cache_entry *ce;
+	int pos;
+	int namelen = strlen(filename);
+	int fullnamelen;
+	char *fullname;
+
+	if (!prefix)
+		prefix = "";
+
+	/* Wrong stage number? */
+	pos = cache_name_pos(filename, namelen);
+	if (pos < 0)
+		pos = -pos - 1;
+	ce = active_cache[pos];
+	if (ce_namelen(ce) == namelen &&
+	    !memcmp(ce->name, filename, namelen))
+		die("Path '%s' is in the index, but not at stage %d.\n"
+		    "Did you mean ':%d:%s'?",
+		    filename, stage,
+		    ce_stage(ce), filename);
+
+	/* Confusion between relative and absolute filenames? */
+	fullnamelen = namelen + strlen(prefix);
+	fullname = xmalloc(fullnamelen + 1);
+	strcpy(fullname, prefix);
+	strcat(fullname, filename);
+	pos = cache_name_pos(fullname, fullnamelen);
+	if (pos < 0)
+		pos = -pos - 1;
+	ce = active_cache[pos];
+	if (ce_namelen(ce) == fullnamelen &&
+	    !memcmp(ce->name, fullname, fullnamelen))
+		die("Path '%s' is in the index, but not '%s'.\n"
+		    "Did you mean ':%d:%s'?",
+		    fullname, filename,
+		    ce_stage(ce), fullname);
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in the index.", filename);
+	if (errno == ENOENT || errno == ENOTDIR)
+		die("Path '%s' does not exist (neither on disk nor in the index).",
+		    filename);
+
+	free(fullname);
+}
+
+
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
@@ -850,6 +939,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 			}
 			pos++;
 		}
+		if (!gently)
+			diagnose_invalid_index_path(stage, prefix, cp);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -862,9 +953,25 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
-			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      mode);
+		char *object_name;
+		if (!gently) {
+			object_name = xmalloc(cp-name+1);
+			strncpy(object_name, name, cp-name);
+			object_name[cp-name] = '\0';
+		}
+		if (!get_sha1_1(name, cp-name, tree_sha1)) {
+			const char *filename = cp+1;
+			ret = get_tree_entry(tree_sha1, filename, sha1, mode);
+			if (!gently) {
+				diagnose_invalid_sha1_path(prefix, filename,
+							   tree_sha1, object_name);
+				free(object_name);
+			}
+			return ret;
+		} else {
+			if (!gently)
+				die("Invalid object name '%s'.", object_name);
+		}
 	}
 	return ret;
 }
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
new file mode 100755
index 0000000..af721f9
--- /dev/null
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='test git rev-parse diagnosis for invalid argument'
+
+exec </dev/null
+
+. ./test-lib.sh
+
+HASH_file=
+
+test_expect_success 'set up basic repo' '
+	echo one > file.txt &&
+	mkdir subdir &&
+	echo two > subdir/file.txt &&
+	echo three > subdir/file2.txt &&
+	git add . &&
+	git commit -m init &&
+	echo four > index-only.txt &&
+	git add index-only.txt &&
+	echo five > disk-only.txt
+'
+
+test_expect_success 'correct file objects' '
+	HASH_file=$(git rev-parse HEAD:file.txt) &&
+	git rev-parse HEAD:subdir/file.txt &&
+	git rev-parse :index-only.txt &&
+	(cd subdir &&
+	 git rev-parse HEAD:subdir/file2.txt &&
+	 test $HASH_file = $(git rev-parse HEAD:file.txt) &&
+	 test $HASH_file = $(git rev-parse :file.txt) &&
+	 test $HASH_file = $(git rev-parse :0:file.txt) )
+'
+
+test_expect_success 'incorrect revision id' '
+	test_must_fail git rev-parse foobar:file.txt 2>error &&
+	grep "Invalid object name '"'"'foobar'"'"'." error &&
+	test_must_fail git rev-parse foobar 2> error &&
+	grep "unknown revision or path not in the working tree." error
+'
+
+test_expect_success 'incorrect file in sha1:path' '
+	test_must_fail git rev-parse HEAD:nothing.txt 2> error &&
+	grep "fatal: Path '"'"'nothing.txt'"'"' does not exist in '"'"'HEAD'"'"'" error &&
+	test_must_fail git rev-parse HEAD:index-only.txt 2> error &&
+	grep "fatal: Path '"'"'index-only.txt'"'"' exists on disk, but not in '"'"'HEAD'"'"'." error &&
+	(cd subdir &&
+	 test_must_fail git rev-parse HEAD:file2.txt 2> error &&
+	 grep "Did you mean '"'"'HEAD:subdir/file2.txt'"'"'?" error )
+'
+
+test_expect_success 'incorrect file in :path and :N:path' '
+	test_must_fail git rev-parse :nothing.txt 2> error &&
+	grep "fatal: Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
+	test_must_fail git rev-parse :1:nothing.txt 2> error &&
+	grep "Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
+	test_must_fail git rev-parse :1:file.txt 2> error &&
+	grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
+	(cd subdir &&
+	 test_must_fail git rev-parse :1:file.txt 2> error &&
+	 grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
+	 test_must_fail git rev-parse :file2.txt 2> error &&
+	 grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error &&
+	 test_must_fail git rev-parse :2:file2.txt 2> error &&
+	 grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error) &&
+	test_must_fail git rev-parse :disk-only.txt 2> error &&
+	grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error
+'
+
+test_done
-- 
1.6.6.rc0.355.gcb53a.dirty

^ permalink raw reply related

* Re: Approach for collaborative branches
From: Gary Pickrell @ 2009-12-06 18:56 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqhbs4f2gz.fsf@bauges.imag.fr>


Thank you Matthieu,

It did exactly what I wanted and I wouldn't have figured it out by myself.

    -Gary


Matthieu Moy wrote:
> Gary Pickrell <gary@pickrell.com> writes:
>
>   
>> 1) Clone the repository on Ubuntu
>> 2) Made an Ubuntu branch
>> 3) Made my changes to the code.  Added files...ect
>> 4) Used git push origin Ubuntu to push the changes to the repository
>>
>> I'm unable to see my Ubuntu changes on my windows machine.  How should
>> I proceed?
>>     
>
> On the windows machine, you should do first
>
>   git fetch
>
> this will tell you about the new branch if you didn't fetch it
> already. If it's called Ubuntu remotely, it's probably called
> origin/Ubuntu on your local repository after doing a fetch. Therefore,
> you can now do
>
>   git merge origin/Ubuntu
>
>
> Now, you probably also want your local branch to be named the same way
> on both machines. One way to do that is to create a branch "foo" on
> the repository, then on both sides, fetch it and do
>
>   git checkout --track origin/foo
>
> this will create a local branch foo, and tell git that further "pull"
> should take their changes from origin/foo.
>
>   

^ permalink raw reply

* Re: [PATCH 1/2] Documentation: 'git add --all' can remove files
From: Junio C Hamano @ 2009-12-06 21:14 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git
In-Reply-To: <4B1BF2D2.8000506@gmail.com>

Björn Gustavsson <bgustavsson@gmail.com> writes:

> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index e93e606..b7e8aa2 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -16,6 +16,8 @@ DESCRIPTION
>  -----------
>  This command adds the current content of new or modified files to the
>  index, thus staging that content for inclusion in the next commit.
> +(Given the `--all` option, the `git add` command will also remove files
> +from the index that are no longer present in the working tree.)

The first paragraph says what "git add" does in general (does "something"
to the index) and continues to the second paragraph that describes what an
index is.

It is a good point that you noticed that the existing description of 'does
"something" to the index' covers only the "add as an entirety" and not the
whole range of what "git add" can be used for.  One thing that is missing
is a removal, and "-A" is one of the two options that lets you do so.

But "-u" is to match the index with the work tree contents (it does not
add untracked-so-far paths).  At least, "Given the `--all` option," part
needs to be rethought.

More importantly, 'does "something" to the index' is not limited to the
"add whole" and "remove path".  Perhaps we would want to update the first
paragraph like this to cover partial add while we are at it.

	This command updates the index using the current content found in
        the work tree, to prepare the content staged for the next commit.
        It typically adds the current content of existing paths as a whole
        but with some options, it can also be used to add a content with
        only part of the changes made to the work tree files applied, or
        remove paths that do not exist in the work tree anymore.

That way we also cover what "add -p" lets you do.

> ++
> +This option is especially useful after running the `rsync` command with
> +the working tree as the target to sync the index with the working tree.

I do not think mentioning `rsync` like this is helpful; I actually think
this clutteres the document and nudges people in a wrong direction at the
same time.  You usually don't overwrite your source tree you already have
under control with rsync.  Use cases that do not use "git" as a SCM but as
something else do have a valid reason to use rsync, but "git add -A" being
useful for that use case is a side effect, and I do not think we should
hint people who use "git" as a SCM into that direction, by casually
mentioning the use of `rsync` like the above as if it is a common and
encouraged thing to update your work tree from sideways.

> diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
> index 5afb1e7..b4dff5b 100644
> --- a/Documentation/git-rm.txt
> +++ b/Documentation/git-rm.txt
> @@ -81,6 +81,10 @@ two directories `d` and `d2`, there is a difference between
>  using `git rm \'d\*\'` and `git rm \'d/\*\'`, as the former will
>  also remove all of directory `d2`.
>  
> +To automatically remove all files from the index that are no longer
> +present in the working tree, see the `--all` option for
> +linkgit:git-add[1].

I think this change is a lot more harmful than being helpful.  Many lazy
readers would _not_ read "git add --help" but would think "git add -A"
would "automatically remove all files from the index that are no longer
present".  But "add -A" does a lot more than:

    git diff --name-only --diff-filter=D -z | xargs -0 rm -f

So overall I am happy with the issue you identified, but not very happy
with the proposed solution.

^ permalink raw reply

* Re: [StGit PATCH v2 0/6] add support for git send-email
From: Catalin Marinas @ 2009-12-06 22:16 UTC (permalink / raw)
  To: Alex Chiang; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

2009/12/2 Alex Chiang <achiang@hp.com>:
> This is v2 of the series that starts teaching stg mail how to
> call git send-email.

I merged these patches into the "proposed" branch for now and test
them for a bit more. It would be nice for some of the stgit options to
be translated into git send-email options (I don't mind renaming the
stgit options to match the git ones):

--refid -> --in-reply-to
--noreply -> --no-thread

Thanks.

-- 
Catalin

^ permalink raw reply

* [PATCH 1/2] Documentation: 'git add -A' can remove files
From: Björn Gustavsson @ 2009-12-06 23:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Document that 'git add -A/--all' can remove files no longer
present in the working tree from the index, and also document the
behavior with and without path arguments on the command line.

Also update the intro paragraph (as suggested by Junio, with
some minor edits) to make it clear that 'git add' is able to
delete and to also cover the -p option.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Thanks, Junio, for reviewing my patch, and for suggesting a
much better intro paragraph.

I have cut out the mention of rsync and the reference from
git-rm.

I also realized that the documentation was not clear on whether
any path arguments were allowed, so I have clarified that too.

 Documentation/git-add.txt |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index e93e606..6d3e76f 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -14,8 +14,12 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-This command adds the current content of new or modified files to the
-index, thus staging that content for inclusion in the next commit.
+This command updates the index using the current content found in
+the working tree, to prepare the content staged for the next commit.
+It typically adds the current content of existing paths as a whole,
+but with some options it can also be used to add content with
+only part of the changes made to the working tree files applied, or
+remove paths that do not exist in the work tree anymore.
 
 The "index" holds a snapshot of the content of the working tree, and it
 is this snapshot that is taken as the contents of the next commit.  Thus
@@ -102,10 +106,14 @@ apply.
 
 -A::
 --all::
-	Update files that git already knows about (same as '\--update')
-	and add all untracked files that are not ignored by '.gitignore'
-	mechanism.
-
+	Update files that git already knows about (same as '\--update'),
+	add all untracked files that are not ignored by the '.gitignore'
+	mechanism and remove files from the index that are no longer
+	present in the working tree.
++
+If no paths are given on the command line, `git add -A` will operate
+on the current directory and its subdirectories. If paths are given,
+it will operate on those paths and their subdirectories.
 
 -N::
 --intent-to-add::
-- 
1.6.6.rc1.31.g1a56b

^ permalink raw reply related

* [PATCH 2/2] git-add/rm doc: Consistently back-quote
From: Björn Gustavsson @ 2009-12-06 23:04 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Consistently back-quote commands, options and file names.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
 Documentation/git-add.txt |   30 +++++++++++++++---------------
 Documentation/git-rm.txt  |    6 +++---
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 6d3e76f..4e33b30 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -24,22 +24,22 @@ remove paths that do not exist in the work tree anymore.
 The "index" holds a snapshot of the content of the working tree, and it
 is this snapshot that is taken as the contents of the next commit.  Thus
 after making any changes to the working directory, and before running
-the commit command, you must use the 'add' command to add any new or
+the commit command, you must use the `add` command to add any new or
 modified files to the index.
 
 This command can be performed multiple times before a commit.  It only
 adds the content of the specified file(s) at the time the add command is
 run; if you want subsequent changes included in the next commit, then
-you must run 'git add' again to add the new content to the index.
+you must run `git add` again to add the new content to the index.
 
-The 'git status' command can be used to obtain a summary of which
+The `git status` command can be used to obtain a summary of which
 files have changes that are staged for the next commit.
 
-The 'git add' command will not add ignored files by default.  If any
-ignored files were explicitly specified on the command line, 'git add'
+The `git add` command will not add ignored files by default.  If any
+ignored files were explicitly specified on the command line, `git add`
 will fail with a list of ignored files.  Ignored files reached by
 directory recursion or filename globbing performed by Git (quote your
-globs before the shell) will be silently ignored.  The 'add' command can
+globs before the shell) will be silently ignored.  The `add` command can
 be used to add ignored files with the `-f` (force) option.
 
 Please see linkgit:git-commit[1] for alternative ways to add content to a
@@ -99,15 +99,15 @@ apply.
 	Update only files that git already knows about, staging modified
 	content for commit and marking deleted files for removal. This
 	is similar
-	to what "git commit -a" does in preparation for making a commit,
+	to what `git commit -a` does in preparation for making a commit,
 	except that the update is limited to paths specified on the
 	command line. If no paths are specified, all tracked files in the
 	current directory and its subdirectories are updated.
 
 -A::
 --all::
-	Update files that git already knows about (same as '\--update'),
-	add all untracked files that are not ignored by the '.gitignore'
+	Update files that git already knows about (same as `\--update`),
+	add all untracked files that are not ignored by the `.gitignore`
 	mechanism and remove files from the index that are no longer
 	present in the working tree.
 +
@@ -120,8 +120,8 @@ it will operate on those paths and their subdirectories.
 	Record only the fact that the path will be added later. An entry
 	for the path is placed in the index with no content. This is
 	useful for, among other things, showing the unstaged content of
-	such files with 'git diff' and committing them with 'git commit
-	-a'.
+	such files with `git diff` and committing them with `git commit
+	-a`.
 
 --refresh::
 	Don't add the file(s), but only refresh their stat()
@@ -141,7 +141,7 @@ it will operate on those paths and their subdirectories.
 Configuration
 -------------
 
-The optional configuration variable 'core.excludesfile' indicates a path to a
+The optional configuration variable `core.excludesfile` indicates a path to a
 file containing patterns of file names to exclude from git-add, similar to
 $GIT_DIR/info/exclude.  Patterns in the exclude file are used in addition to
 those in info/exclude.  See linkgit:gitrepository-layout[5].
@@ -189,7 +189,7 @@ and type return, like this:
     What now> 1
 ------------
 
-You also could say "s" or "sta" or "status" above as long as the
+You also could say `s` or `sta` or `status` above as long as the
 choice is unique.
 
 The main command loop has 6 subcommands (plus help and quit).
@@ -197,9 +197,9 @@ The main command loop has 6 subcommands (plus help and quit).
 status::
 
    This shows the change between HEAD and index (i.e. what will be
-   committed if you say "git commit"), and between index and
+   committed if you say `git commit`), and between index and
    working tree files (i.e. what you could stage further before
-   "git commit" using "git-add") for each path.  A sample output
+   `git commit` using `git add`) for each path.  A sample output
    looks like this:
 +
 ------------
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 5afb1e7..d4162f6 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -12,13 +12,13 @@ SYNOPSIS
 DESCRIPTION
 -----------
 Remove files from the index, or from the working tree and the index.
-'git-rm' will not remove a file from just your working directory.
+`git rm` will not remove a file from just your working directory.
 (There is no option to remove a file only from the work tree
 and yet keep it in the index; use `/bin/rm` if you want to do that.)
 The files being removed have to be identical to the tip of the branch,
 and no updates to their contents can be staged in the index,
 though that default behavior can be overridden with the `-f` option.
-When '--cached' is given, the staged content has to
+When `--cached` is given, the staged content has to
 match either the tip of the branch or the file on disk,
 allowing the file to be removed from just the index.
 
@@ -64,7 +64,7 @@ OPTIONS
 
 -q::
 --quiet::
-	'git-rm' normally outputs one line (in the form of an "rm" command)
+	`git rm` normally outputs one line (in the form of an `rm` command)
 	for each file removed. This option suppresses that output.
 
 
-- 
1.6.6.rc1.31.g1a56b

^ permalink raw reply related

* Re: [PATCH 1/2] Documentation: 'git add -A' can remove files
From: Junio C Hamano @ 2009-12-06 23:31 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git
In-Reply-To: <4B1C384A.8000106@gmail.com>

Björn Gustavsson <bgustavsson@gmail.com> writes:

> Document that 'git add -A/--all' can remove files no longer
> present in the working tree from the index, and also document the
> behavior with and without path arguments on the command line.

Thanks.

> +This command updates the index using the current content found in
> +the working tree, to prepare the content staged for the next commit.
> +It typically adds the current content of existing paths as a whole,
> +but with some options it can also be used to add content with
> +only part of the changes made to the working tree files applied, or
> +remove paths that do not exist in the work tree anymore.

You probably want to change the last one also to "working tree"?

I often write this as "work tree" without thinking too much about "work"
vs "working", but if anybody asks me, I prefer the former because it is
shorter and because it is more consistent with the way how names of the
environment variable GIT_WORK_TREE and the configuration variable
core.worktree are spelled.  I personally am OK with either word used in
the descriptive text, as there is no room for confusion.

But it would be better to be consistent in a single paragraph.

>  -A::
>  --all::
> -	Update files that git already knows about (same as '\--update')
> -	and add all untracked files that are not ignored by '.gitignore'
> -	mechanism.
> -
> +	Update files that git already knows about (same as '\--update'),
> +	add all untracked files that are not ignored by the '.gitignore'
> +	mechanism and remove files from the index that are no longer
> +	present in the working tree.
> ++
> +If no paths are given on the command line, `git add -A` will operate
> +on the current directory and its subdirectories. If paths are given,
> +it will operate on those paths and their subdirectories.

The first line of the existing description for "--all", by saying "same as
--update", refers to the first sentence of the corresponding entry for
"update", which says:

    -u::
    --update::
            Update only files that git already knows about, staging modified
            content for commit and marking deleted files for removal. This
            is similar
            to what "git commit -a" does in preparation for making a commit,
            except that the update is limited to paths specified on the
            command line. If no paths are specified, all tracked files in the
            current directory and its subdirectories are updated.

In fact, "-A" is "do everything -u does, including removal of lost paths,
honoring the pathspecs exactly the same way (e.g. no pathspec is to work
in the current directory). but unlike -u, also add any new files that are
not excluded by the ignore mechanism."  There is something wrong if we
have to spend more lines to describe "-A" than we describe "-u", if
description of "-A" says "it does the same for -u, and in addition...".

I wonder if we can restructure the description of "-u" to make it easier
to read, to simplify the description of "-A".

^ permalink raw reply

* Re: Continued work on sr/vcs-helper and sr/gfi-options
From: Junio C Hamano @ 2009-12-06 23:36 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List
In-Reply-To: <fabb9a1e0912060223h148a67b0q589b8461dae6330e@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

>>  - Merge today's 'master' to sr/remote-hg (optional);
>
> I'm not sure why this is though? (no objections against it, I just
> don't understand the motivation)

It would make sense if you use 1.6.6 features in your new series (as the
forkpoint of sr/vcs-helper is beginning to look a tad stale), but
otherwise unnecessary; that is the reason why I said it is "(optional)"
and it is up to what is in the remote-hg patch.

>>  - Create an unstable sr/pu-remote-hg branch that:
>>
>>   - is reset to the tip of sr/remote-hg at the beginning of the day;
>>   - merges the day's sr/gfi-options on top;
>>   - re-applies patches to implement Hg interoperation on top of the
>>     result.
>
> Ok, that does make sense, how would I send out patches for review from
> this unstable branch though? (since others would not have the required
> merges etc) I reckon it would be necessary to publish sr/pu-remote-hg
> somewhere?

I was thinking about carrying it myself (perhaps with help from you in
conflict resolution as necessary) when I wrote it, but if you want me to
pull from your repository somewhere e.g. repo.or.cz, that would also be
fine.

^ permalink raw reply

* Re: clang static analyzer
From: Nicolas Sebrecht @ 2009-12-06 23:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Nicolas Pitre, Tomas Carnecky, git list, Nicolas Sebrecht
In-Reply-To: <20091206160436.GA7140@coredump.intra.peff.net>

The 06/12/09, Jeff King wrote:
> On Sun, Dec 06, 2009 at 10:39:56AM -0500, Nicolas Pitre wrote:
> 
> > >   2. If it is a false positive, see what it would take to silence clang
> > >      and submit a patch.  I don't think we are opposed to annotations
> > >      that help analysis tools as long as those annotations aren't too
> > >      intrusive or make the code less readable.
> > 
> > I'm a bit skeptical here.  Going down that route might mean that we'll 
> > eventually have to add all sort of crap to accommodate everyone's 
> > preferred static analysis tool of the day.  Would be far nicer to try to 
> > make those tools more intelligent instead, or at least make them 
> > understand an out-of-line annotation format that does not clutter the 
> > code itself.
> 
> To be clear, I am a bit skeptical, too. 

Me too. I have no idea about how clang works but if there are enough
work done to support such a tool in the code itself, it would be sad to
not share and promote this work. If Junio cares enough himself, he could
set up a public dedicated branch. Now if he doesn't, it's not necessary
at all. Tomas or anyone else with enough time and motivation can fork
the repository for this purpose.

The latter option would be good and appreciated by everybody here, I
think.

-- 
Nicolas Sebrecht

^ permalink raw reply

* Re: [PATCH 1/2] Documentation: 'git add -A' can remove files
From: Junio C Hamano @ 2009-12-07  0:05 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git
In-Reply-To: <4B1C384A.8000106@gmail.com>

Björn Gustavsson <bgustavsson@gmail.com> writes:

> I have cut out the mention of rsync and the reference from
> git-rm.

I am sorry, but I would like to rescind my earlier comment on the 'rsync'
issue.  When tracking "vendor code drop", you often need to:

    - untar the version 1 of vendor code drop
    - git init
    - git add -A
    - git commit -m 'vendor #1'
    - git ls-files -z | xargs -0 rm -f ;# remove everything
    - untar the version 1.1
    - git add -A
    - git commit -m 'vendor #1.1'
    ...

and the ability to notice and remove "gone" files while adding anything
new indeed is a very valid and useful thing to have.  The initial "-A" can
also be spelled as "git add .", but not the second and subsequent code
drop.

If your vendor tree is online and rsync'able, "untar" in the above
sequence will naturally be replaced with "rsync", and it is entirely
within the scope of SCM.

Regarding "git rm", if people very often need to remove paths from the
index that are already gone from the filesystem, perhaps we would need an
option to "rm" to let them do that.

However, if the reason these people want to do such a removal is almost
always because they are accepting a vendor code drop (meaning, they not
only want to remove disappeared paths but also want to add new paths at
the same time), referring to "git add -A" from that manual page would make
a lot of sense.  So in that sense, I am not against your original patch
per-se, but I think the prerequisite context needs to be explained in the
documentation a bit better.

Here is my attempt.  I didn't check the existing text of "git rm" manual,
so I do not know if its structure to use displayed examples flows
naturally with the existing text; you may need to rewrite to adjust to the
existing style.

--

Sometimes people ask how paths that disappeared from the filesystem can be
removed from the index.  A straight answer is

----------------
git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached
----------------

but it often is not what these people really want to do (XY problem), and
there are two alternate answers that may suit their situation better.

1. They may be doing their own development and have removed files from the
filesystem using "rm" (not "git rm"), fully intending that they want to
record the removal of these paths in the next commit.  If you make the
next commit with "git commit -a", it will automatically notice these
removals, and there is no need for such a removal they ask in the first
place, as long as they intend to make a full commit.

Asking for "removing all the removed paths from the index" is a sign of
wanting to commit the full state of the work tree, as opposed to "commit
this and that change but not others, which you do not use "commit -a" for.

So the second answer may be "you do not need to in your use case."

2. They may be accepting a new vendor code drop, and have just updated
their work tree by untarring (or rsync'ing).  They not only want to record
removal of disappeared paths, but also want to record addition of new
paths, but if they know only "git add" (but not "add -A" or "add -u"), it
may appear that a separate "git rm" to remove disappeared paths is needed.
In such a case, instead of doing

----------------
git add .
git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached
----------------

they can simply do so with

----------------
git add -A
----------------

Hence the third answer may be "you do not have to in your use case."

^ permalink raw reply

* Re: clang static analyzer
From: Junio C Hamano @ 2009-12-07  0:18 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: git list
In-Reply-To: <33ABC714-2BCC-4910-BCAE-D331AAF2A724@dbservice.com>

Tomas Carnecky <tom@dbservice.com> writes:

> pretty.c:get_header() - if 'line' doesn't contain a newline character,
> line is set to NULL on first iteration and then passed to strchr() in
> the second itration.

Thanks.

In practice, we will always have a newline, as we are reading from a valid
commit object in this codepath.


 pretty.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/pretty.c b/pretty.c
index 8f5bd1a..7eb5384 100644
--- a/pretty.c
+++ b/pretty.c
@@ -245,11 +245,11 @@ static char *get_header(const struct commit *commit, const char *key)
 	int key_len = strlen(key);
 	const char *line = commit->buffer;
 
-	for (;;) {
+	while (line) {
 		const char *eol = strchr(line, '\n'), *next;
 
 		if (line == eol)
-			return NULL;
+			break;
 		if (!eol) {
 			eol = line + strlen(line);
 			next = NULL;
@@ -262,6 +262,7 @@ static char *get_header(const struct commit *commit, const char *key)
 		}
 		line = next;
 	}
+	return NULL;
 }
 
 static char *replace_encoding_header(char *buf, const char *encoding)

^ permalink raw reply related

* Re: clang static analyzer
From: Junio C Hamano @ 2009-12-07  0:26 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: git list
In-Reply-To: <33ABC714-2BCC-4910-BCAE-D331AAF2A724@dbservice.com>

Tomas Carnecky <tom@dbservice.com> writes:

> xdiff-interface.c:xdiff_set_find_func() - When 'value' is a string with
> no newline character in it, the loop at line 291 sets 'value' to NULL on
> its first iteration and then passes 'value' to strchr() in the second
> iteration.

Thanks, but I am confused with your analysis.

If value doesn't have '\n', then regs->nr is 1 when you go into the loop
at ll. 291-, because we counted the number of LF in the first loop in the
function.

The first iteration of the second loop sets ep to NULL, expression is set
to value, then we run regcomp on the expression.  Then at the end of the
loop we do set value to a bogus "(char*)1".  But incrementing i makes it
go over regs->nr and satisfy the termination condition of the loop; we
happily exit the loop before we use the now bogus "value".

^ permalink raw reply

* Re: [PATCH 1/2] builtin-commit: refactor short-status code into wt-status.c
From: Junio C Hamano @ 2009-12-07  0:30 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <c42e3f38816a08aba4610cb808f09067a07ad097.1260025135.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Currently, builtin-commit.c contains most code producing the
> short-status output, whereas wt-status.c contains most of the code for
> the long format.
>
> Refactor so that most of the long and short format producing code
> resides in wt-status.c and is named analogously.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  builtin-commit.c |  101 ++---------------------------------------------------
>  wt-status.c      |   89 +++++++++++++++++++++++++++++++++++++++++++++++
>  wt-status.h      |    2 +
>  3 files changed, 95 insertions(+), 97 deletions(-)

Nice.

Thanks.

^ permalink raw reply

* Re: [PATCH] pull: clarify advice for the unconfigured error case
From: Junio C Hamano @ 2009-12-07  0:29 UTC (permalink / raw)
  To: Jan Krüger
  Cc: Junio C Hamano, Jonathan Nieder, Jeff King, Jan Nieuwenhuizen,
	Tomas Carnecky, git list
In-Reply-To: <20091203115110.08dde406@perceptron>

Jan Krüger <jk@jk.gs> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>
>> Is this a good replacement for 31971b3 (git-pull.sh --rebase: overhaul
>> error handling when no candidates are found, 2009-11-12) that is on
>> 'pu' and does the lack of follow-up mean everybody involved in the
>> discussion is happy with this version?
>
> I'm not deliriously happy but I can't think of a better version, and I
> suppose it's better than what I suggested. In other words, I'm fine
> with the patch.

Thanks.

^ permalink raw reply

* Re: [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: Junio C Hamano @ 2009-12-07  0:30 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqhbs4dkjr.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>>> +extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix);
>>
>> Do I understand correctly that "fatal" here is the same as "!gently"
>> elsewhere in the API?
>
> It seems it is. I renamed it.

This was a pure question, not a suggestion to change it (we do name a
function do_foo_gently() when there is do_foo() that does the same but
reports errors more noisily, though).  I found the name "fatal" a bit
confusing as I at first couldn't tell if the caller was telling the
function that it already detected a "fatal" error (and telling the
function to report the fatalness) and didn't realize that the caller is
instead saying "if you find an error, treat it as a fatal one" until I
read it again.

I am Ok with the new "gently" name with the negative semantics as well (I
see no need to change it back to "error_is_fatal").

Thanks.

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.6.5.4
From: Junio C Hamano @ 2009-12-07  0:30 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: Andreas Schwab, Michael J Gruber, git
In-Reply-To: <20091204193355.GC4629@inocybe.localdomain>

Todd Zullinger <tmz@pobox.com> writes:

> Something like this perhaps?  I tested it with 1.6.5.4 on Fedora 10,
> 12 and CentOS 5.4 but it could surely use more eyes and testing to
> ensure it works as it should and doesn't have unintended negative
> effects on make clean and such.

Thanks (and thanks Andreas for pointing out a yet another distro that is
different).  I think the patch makes sense.

> This does set MAN_BASE_URL unconditionally, pointing to kernel.org.

I'd change this to point at "file://$(htmldir)/", though.

^ permalink raw reply

* Re: Continued work on sr/vcs-helper and sr/gfi-options
From: Sverre Rabbelier @ 2009-12-07  0:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vfx7nekuf.fsf@alter.siamese.dyndns.org>

Heya,

On Mon, Dec 7, 2009 at 00:36, Junio C Hamano <gitster@pobox.com> wrote:
> It would make sense if you use 1.6.6 features in your new series (as the
> forkpoint of sr/vcs-helper is beginning to look a tad stale), but
> otherwise unnecessary; that is the reason why I said it is "(optional)"
> and it is up to what is in the remote-hg patch.

Gotcha, I don't think I use any 1.6.6 features though, so I don't
think that will be necessary.

> I was thinking about carrying it myself (perhaps with help from you in
> conflict resolution as necessary) when I wrote it

That would be great! If you are you offering to create the
sr/pu-remote-hg branch and merge it to pu so that I can send patches
based on that, please do. If not, what do I do instead? :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH 1/2] Documentation: 'git add -A' can remove files
From: Björn Steinbrink @ 2009-12-07  0:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Björn Gustavsson, git
In-Reply-To: <7vr5r7el2q.fsf@alter.siamese.dyndns.org>

On 2009.12.06 15:31:25 -0800, Junio C Hamano wrote:
> I wonder if we can restructure the description of "-u" to make it easier
> to read, to simplify the description of "-A".

What I usually say on #git is something like:

	"git add <path>" looks at the working tree to find files
	matching <path>.  "git add -u <path>" looks at the index, and
	"git add -A <path>" looks at both. Therefore "add" and "add -A"
	can add new files to the index, and "add -u" and "add -A" can
	remove files from it.

	And for convenience, -u and -A default to "." as the path argument.

So maybe something like this?

-u, --update
    Instead of matching <filepattern> against files in the working tree,
    it is matched against the already tracked files in the index. This
    means that it won't find any new files, but can find files already
    deleted from the working tree and remove them from the index. Also,
    if no <filepattern> is given, this option will make it default to
    ".", updating all tracked files in the current directory and its
    subdirectories.

-A, --all
    Like -u, but matches <filepattern> against files in the index in
    addition to the files in working tree. This means that it can find
    new files as well.


Björn

^ permalink raw reply

* Re: [PATCH] gitweb.js: Harden setting blamed commit info in incremental blame
From: Stephen Boyd @ 2009-12-07  1:04 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200911251536.08993.jnareb@gmail.com>

On Wed, 2009-11-25 at 15:36 +0100, Jakub Narebski wrote:
> Well, the one time I was able to run debugger (F12, select 'Script', select
> 'gitweb.js') with error occurring and without IE hanging (for README file)
> it did show an error for the following line:
> 
>   if (xhr.readyState === 3 && xhr.status !== 200) {
> 
> When I checked 'xhr' object, it has "Unknown error" as contents of 
> xhr.statusText field and as contents of xhr.status (sic!), which should
> be a number: HTTP status code.
> 
> Unfortunately I had to take a break... and I was not able to reproduce this
> (without hanging web browser: "program not responding") since then...
> 

Ok. It's December and I've had some more time to look into this.
Initializing 'xhr' to null seems to get rid of the "Unknown error"
problem (see patch below).

The responsiveness on IE8 is pretty poor. I load the page and then after
some waiting (usually 20-30 seconds including IE becoming a "Not
Responding" program) the whole page will load. There is nothing
incremental about it. I'm trying to figure out why it's slow now.

--->8----

Subject: [PATCH] gitweb.js: workaround IE8 "Unknown error"

Internet Explorer 8 complains about an "Unknown error on line 782, char 2".
That line is a reference to xhr and the status code. By initializing xhr
to null this error goes away.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 gitweb/gitweb.js |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
index 2a25b7c..b936635 100644
--- a/gitweb/gitweb.js
+++ b/gitweb/gitweb.js
@@ -126,7 +126,7 @@ function createRequestObject() {
 /* ============================================================ */
 /* utility/helper functions (and variables) */

-var xhr;        // XMLHttpRequest object
+var xhr = null;        // XMLHttpRequest object
 var projectUrl; // partial query + separator ('?' or ';')

 // 'commits' is an associative map. It maps SHA1s to Commit objects.
-- 
1.6.6.rc1.39.g9a42

^ permalink raw reply related

* Re: [PATCH] gitweb.js: Harden setting blamed commit info in incremental blame
From: Stephen Boyd @ 2009-12-07  1:19 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <1260147860.1579.47.camel@swboyd-laptop>

On Sun, 2009-12-06 at 17:04 -0800, Stephen Boyd wrote:
> 
> Ok. It's December and I've had some more time to look into this.
> Initializing 'xhr' to null seems to get rid of the "Unknown error"
> problem (see patch below).
> 

Ah sorry. Seems this doesn't work and I was just getting lucky.

^ permalink raw reply

* Re: Unapplied patches reminder
From: Greg Price @ 2009-12-07  3:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Geoffrey Thomas, git, Nanako Shiraishi
In-Reply-To: <7voco4gtxd.fsf@alter.siamese.dyndns.org>

Junio wrote:
> > From: Geoffrey Thomas <geofft@mit.edu>
> > Subject: [PATCH] diffcore-order: Default the order file to .git/info/order.
> > Date: Sat, 12 Sep 2009 19:49:48 -0400
> > Message-ID: <1252799388-16295-1-git-send-email-geofft@mit.edu>
> >
> >     Since order files tend to be useful for all operations in the
> >     project/repository, add a default location for the order file, so that
> >     you don't have to specify -O<orderfile> on every diff or similar
> >     operation.
>
> Except that "$GIT_DIR/info/order" is a bit too generic a name ("eh,
> 'order'?  Order of what?"), I do not think this will hurt, as no existing
> repositories would have such a file that would cause any behaviour change
> to existing users.  The reason I did not queue it was because there wasn't
> any discussion on it (not even the filename being too generic) which was
> an indication that not many people are interested in such a feature.
>
> That of course can be remedied by interested people speaking out.

I never use the -O option, but if this feature were available I think
I would use it.  It would make it possible to configure a helpful
order once -- for example, putting a changelog file first in order to
compare it with the commit message -- and then always use it without
effort.

A less generic name might be 'difforder'.

Greg

^ permalink raw reply

* [PATCHv2 0/2] Add a "fixup" command to "rebase --interactive"
From: Michael Haggerty @ 2009-12-07  4:22 UTC (permalink / raw)
  To: git; +Cc: gitster, git, Johannes.Schindelin, bgustavsson, Michael Haggerty

Thanks, everybody, for all of the feedback.  This is the redone patch
series, which I think addresses all of your suggestions.

I would still like to implement "don't open the editor if there are
only fixups", but if it's OK I'll work on that in a separate patch
series when I have time.

Michael J Gruber wrote:
> My first reaction to the subject was "Huh? What repository?". So I
> suggest something like
> 
> t3404: Better document the original repository layout
> 
> as a more descriptive subject.

Good idea.  Done.

Johannes Schindelin wrote:
> Alternatively, one could use the test_commit function, I guess. [...]

Yes, that's much easier.  Changed.  This makes the old first and
second patches reduce to a single one.

Johannes Schindelin wrote:
> On Fri, 4 Dec 2009, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>>> index 0bd3bf7..539413d 100755
>>> --- a/git-rebase--interactive.sh
>>> +++ b/git-rebase--interactive.sh
>>> @@ -302,7 +302,7 @@ nth_string () {
>>>  
>>>  make_squash_message () {
>>>  	if test -f "$SQUASH_MSG"; then
>>> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
>>> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>>>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>> This sed replacement worries me.  I don't have a time to check myself
>> today but do we use \(this\|or\|that\) alternates with our sed script
>> already elsewhere in the codebase (test scripts do not count)?
>>
>> Otherwise this may suddenly be breaking a platform that has an
>> implementation of sed that may be substandard but so far has been
>> sufficient to work with git.
> 
> IIRC "|" was not correctly handled by BSD sed (used e.g. in MacOSX).
> 
> So maybe it would be best to just look for "commit message"?  I agree with
> Michael that the regex should not be too loose.

Thanks for pointing this out.  I replaced the problematic part of the
regexp with "[snrt][tdh]", which isn't quite so selective but should
be portable.  (This is the same fix that Junio suggested.)

Björn Gustavsson wrote:
> On Fri, Dec 4, 2009 at 3:36 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> Nitpick: the recommended style is to leave out the full stop
> at the end of the first line of the commit message.

Nit picked.

Junio C Hamano wrote:
> IIRC, the end result of the bikeshedding for the name of the command was
> won by Dscho's "fixup":
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/127923/focus=121820

That's fine with me (the abbreviation is even the same :-) ).
Changed.

Michael

Michael Haggerty (2):
  t3404: Use test_commit to set up test repository
  Add a command "fixup" to rebase --interactive

 Documentation/git-rebase.txt  |   13 +++--
 git-rebase--interactive.sh    |   51 +++++++++++++++++----
 t/lib-rebase.sh               |    7 ++-
 t/t3404-rebase-interactive.sh |   96 +++++++++++++++++++++-------------------
 4 files changed, 103 insertions(+), 64 deletions(-)

^ permalink raw reply


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