* Re: [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: Junio C Hamano @ 2009-12-07 7:27 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqhbs4dkjr.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
The second round still had a few compilation warnings (turned to errors in
my environment) so I fixed them up somewhat. I stopped before getting a
clean compile, though---you will still get:
sha1_name.c: In function 'get_sha1_with_mode_1':
sha1_name.c:956: error: 'object_name' may be used uninitialized in this function
even with this fix-up.
>> 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,...
I thought generally it is accepted that I had a better taste on this list?
;-)
-- squashed --
cache.h | 4 ++--
sha1_name.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index 952bd51..3f9ee86 100644
--- a/cache.h
+++ b/cache.h
@@ -702,11 +702,11 @@ static inline unsigned int hexval(unsigned char c)
#define DEFAULT_ABBREV 7
extern int get_sha1(const char *str, unsigned char *sha1);
-static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
+static inline int 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/sha1_name.c b/sha1_name.c
index 1bc09ac..025244a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -807,7 +807,7 @@ int get_sha1(const char *name, unsigned char *sha1)
/* 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 unsigned char *tree_sha1,
const char *object_name)
{
struct stat st;
^ permalink raw reply related
* Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`
From: Alex Vandiver @ 2009-12-07 7:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd42soo2p.fsf@alter.siamese.dyndns.org>
At Sat Dec 05 21:04:14 -0500 2009, Junio C Hamano wrote:
> Alex Vandiver <alex@chmrr.net> writes:
> > ...
> > "git remote prune [-n | --dry-run] <name>",
> > - "git remote [-v | --verbose] update [-p | --prune] [group | remote]",
> > + "git remote [-v | --verbose] [-q | --quiet] update [-p | --prune] [group]",
Hm, I hadn't noticed that I'd changed "[group | remote]" to "[group]".
I think this is due to a mismerge on my part -- apologies. As another
data point, `git fetch` describes this as "[<repository> | <group>]".
> Three issues to consider:
>
> - shouldn't we use the same typography, i.e. <group>?
>
> - should we say <name> _if_ we are not going to say <group>|<remote>?
>
> - should we keep it as <group>|<remote> to make it clear that only this
> subcommand allows the group nickname?
>
> The first two are easy and I expect the answers to be both yes. The third
> one needs some studying and further thought.
>
> - is "remote update" the only one that takes group nickname?
My quick skim of the code says "yes" -- the other commands only deal
with single remotes at a time, and prune is oblivious to groups.
> - should "remote update" the only one? e.g. does "remote prune" also
> take group? if not, shouldn't it?
Properly, it "ought" to, though I don't see much utility over `git
remote fetch --prune groupname`. Probably at the same time, the
parallel pruning codepaths in builtin-fetch.c:prune_refs() and
builtin-remote.c:prune_remote() should be unified.
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply
* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Junio C Hamano @ 2009-12-07 7:12 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, git, Johannes.Schindelin, bgustavsson
In-Reply-To: <ced6765cff6225a05f196a6896ab577850979ab1.1260099005.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0bd3bf7..a7de5ea 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -302,7 +302,13 @@ 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" \
> + # We want to be careful about matching only the commit
> + # message comment lines generated by this function.
> + # But supposedly some sed versions don't handle "\|"
> + # correctly, so instead of "\(st\|nd\|rd\|th\)", use
> + # the less accurate "[snrt][tdh]" to match the
> + # nth_string endings.
I'd drop this comment; blaming POSIX-compliant sed without GNU extension
is simply wrong.
> + COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
> < "$SQUASH_MSG" | sed -ne '$p')+1))
> echo "# This is a combination of $COUNT commits."
> sed -e 1d -e '2,/^./{
> @@ -315,10 +321,26 @@ make_squash_message () {
> echo
> git cat-file commit HEAD | sed -e '1,/^$/d'
> fi
> - echo
> - echo "# This is the $(nth_string $COUNT) commit message:"
> - echo
> - git cat-file commit $1 | sed -e '1,/^$/d'
> + case $1 in
> + squash)
> + echo
> + echo "# This is the $(nth_string $COUNT) commit message:"
> + echo
> + git cat-file commit $2 | sed -e '1,/^$/d'
> + ;;
> + fixup)
> + echo
> + echo "# The $(nth_string $COUNT) commit message will be skipped:"
> + echo
> + # Comment the lines of the commit message out using
> + # "#" rather than "# " (a) to make them more distinct
> + # from the explanatory comments added by this function
> + # and (b) to make it less likely that the sed regexp
> + # above will be confused by a commented-out commit
> + # message.
Use "# " as prefix and you won't have to worry about a line in the log
message that begins with " Th", no?
In any case, I agree that a comment like this is necessary to warn anybody
who will be touching the code that the COUNT=$((...)) needs to avoid
matching what is produced here, but I find the above 6-line comment a bit
too excessive.
^ permalink raw reply
* Re: [StGit PATCH v2 0/6] add support for git send-email
From: Karl Wiberg @ 2009-12-07 7:09 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Alex Chiang, git
In-Reply-To: <b0943d9e0912061416y4089643l2a5ebdf1c1c0960b@mail.gmail.com>
On Sun, Dec 6, 2009 at 11:16 PM, Catalin Marinas
<catalin.marinas@gmail.com> wrote:
> --refid -> --in-reply-to
> --noreply -> --no-thread
And I must say, the git options are better named than ours anyway ...
--
Karl Wiberg, kha@treskal.com
subrabbit.wordpress.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`
From: Jeff King @ 2009-12-07 6:40 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
In-Reply-To: <1260156352-sup-6170@utwig>
On Mon, Dec 07, 2009 at 01:15:05AM -0500, Alex Vandiver wrote:
> At Sun Dec 06 09:50:00 -0500 2009, Jeff King wrote:
> > Should --quiet also affect this "Updating %s" line?
>
> Looking at it more closely, I think this patch should -- but for
> reasons unrelated o your argument below. The "Updating %s" line there
> is in fetch_remote, which is _only_ called during `git remote add -f`.
> It stands in for the "Fetching %s" line which `git fetch` (and `git
> remote update`) outputs, which (after this patch), _is_ controlled by
> --quiet.
Ah, sorry, this has actually changed since the last time I looked at it
closely, as a result of 9c4a036 (Teach the --all option to 'git fetch',
2009-11-09). So nevermind my complaint...it has actually been addressed
separately (I didn't notice because I have been suppressing the output
by redirecting stdout for some time).
> Thus I feel like "Updating %s" should change to "Fetching %s" for both
> consistency and explicitness, and should also be controlled by
> --quiet. The --quiet-remote option is an entirely different bikeshed,
> which I don't have a strong opinion on, offhand.
Yes, I agree (that it should be quieted, and that it should say
"Fetching"). My --quiet-remote argument is now pointless as of
9c4a036b.
-Peff
^ permalink raw reply
* Re: [PATCH] Allow --quiet option to git remote, particularly for `git remote update`
From: Alex Vandiver @ 2009-12-07 6:15 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091206145000.GC26440@coredump.intra.peff.net>
At Sun Dec 06 09:50:00 -0500 2009, Jeff King wrote:
> Should --quiet also affect this "Updating %s" line?
Looking at it more closely, I think this patch should -- but for
reasons unrelated o your argument below. The "Updating %s" line there
is in fetch_remote, which is _only_ called during `git remote add -f`.
It stands in for the "Fetching %s" line which `git fetch` (and `git
remote update`) outputs, which (after this patch), _is_ controlled by
--quiet.
Thus I feel like "Updating %s" should change to "Fetching %s" for both
consistency and explicitness, and should also be controlled by
--quiet. The --quiet-remote option is an entirely different bikeshed,
which I don't have a strong opinion on, offhand.
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply
* [PATCHv2] tests: handle NO_PYTHON setting
From: Jeff King @ 2009-12-07 5:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Brandon Casey, Johan Herland
Without this, test-lib checks that the git_remote_helpers
directory has been built. However, if we are building
without python, we will not have done anything at all in
that directory, and test-lib's sanity check will fail.
We bump the inclusion of GIT-BUILD-OPTIONS further up in
test-lib; it contains configuration, and as such should be
read before we do any checks (and in this particular case,
we need its value to do our check properly).
Signed-off-by: Jeff King <peff@peff.net>
Looks-fine-to-me-by: Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>
Acked-by: Johan Herland <johan@herland.net>
---
On top of sr/vcs-helper.
Just a repost with acks, as the original seems to have been forgotten in
the ensuing discussion of directory structure.
Makefile | 1 +
t/test-lib.sh | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 42744a4..443565e 100644
--- a/Makefile
+++ b/Makefile
@@ -1743,6 +1743,7 @@ GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
+ @echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
### Detect Tck/Tk interpreter path changes
ifndef NO_TCLTK
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4a40520..2d523fe 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -632,13 +632,15 @@ GIT_CONFIG_NOSYSTEM=1
GIT_CONFIG_NOGLOBAL=1
export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
+. ../GIT-BUILD-OPTIONS
+
GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
export GITPERLLIB
test -d ../templates/blt || {
error "You haven't built things yet, have you?"
}
-if test -z "$GIT_TEST_INSTALLED"
+if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
then
GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
export GITPYTHONLIB
@@ -653,8 +655,6 @@ if ! test -x ../test-chmtime; then
exit 1
fi
-. ../GIT-BUILD-OPTIONS
-
# Test repository
test="trash directory.$(basename "$0" .sh)"
test -n "$root" && test="$root/$test"
--
1.6.6.rc1.292.gd8fe.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] status -s: obey color.status
From: Jeff King @ 2009-12-07 5:26 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <20091207051715.GA17521@coredump.intra.peff.net>
On Mon, Dec 07, 2009 at 12:17:15AM -0500, Jeff King wrote:
> This seems to also turn on color for --porcelain in some cases, because
> git_status_config unconditionally sets s->use_color if you are using
> color.status instead of color.ui. I think we are probably best just
> explicitly disabling options for the "porcelain" format rather than
> trying to come up with some trickery to make sure they never get set.
Also, this means we can hoist repeated code out of the switch statement,
like this:
-- >8 --
Subject: [PATCH] status: reduce duplicated setup code
We have three output formats: short, porcelain, and long.
The short and long formats respect user-config, and the
porcelain one does not. This led to us repeating
config-related setup code for the short and long formats.
Since the last commit, color config is explicitly cleared
when showing the porcelain format. Let's do the same with
relative-path configuration, which enables us to hoist the
duplicated code from the switch statement in cmd_status.
As a bonus, this fixes "commit --dry-run --porcelain", which
was unconditionally setting up that configuration, anyway.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-commit.c | 19 +++++++------------
wt-status.c | 2 ++
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 88b25aa..a11e585 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1018,14 +1018,15 @@ int cmd_status(int argc, const char **argv, const char *prefix)
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
wt_status_collect(&s);
+ if (s.relative_paths)
+ s.prefix = prefix;
+ if (s.use_color == -1)
+ s.use_color = git_use_color_default;
+ if (diff_use_color_default == -1)
+ diff_use_color_default = git_use_color_default;
+
switch (status_format) {
case STATUS_FORMAT_SHORT:
- if (s.relative_paths)
- s.prefix = prefix;
- if (s.use_color == -1)
- s.use_color = git_use_color_default;
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
wt_shortstatus_print(&s, null_termination);
break;
case STATUS_FORMAT_PORCELAIN:
@@ -1033,12 +1034,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
break;
case STATUS_FORMAT_LONG:
s.verbose = verbose;
- if (s.relative_paths)
- s.prefix = prefix;
- if (s.use_color == -1)
- s.use_color = git_use_color_default;
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
wt_status_print(&s);
break;
}
diff --git a/wt-status.c b/wt-status.c
index e9bbfbc..55b6696 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -700,5 +700,7 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination)
void wt_porcelain_print(struct wt_status *s, int null_termination)
{
s->use_color = 0;
+ s->relative_paths = 0;
+ s->prefix = NULL;
wt_shortstatus_print(s, null_termination);
}
--
1.6.6.rc1.292.gd8fe.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] status -s: obey color.status
From: Jeff King @ 2009-12-07 5:17 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <2b987524f57a0ac04e219f82e20e806741ce4eca.1260025135.git.git@drmicha.warpmail.net>
On Sat, Dec 05, 2009 at 04:04:38PM +0100, Michael J Gruber wrote:
> Make the short version of status obey the color.status boolean. We color
> the status letters only, because they carry the state information and are
> potentially colored differently, such as for a file with staged changes
> as well as changes in the worktree against the index.
This seems to also turn on color for --porcelain in some cases, because
git_status_config unconditionally sets s->use_color if you are using
color.status instead of color.ui. I think we are probably best just
explicitly disabling options for the "porcelain" format rather than
trying to come up with some trickery to make sure they never get set.
Like:
-- >8 --
Subject: [PATCH] status: disable color for porcelain format
The porcelain format is identical to the shortstatus format,
except that it should not respect any user configuration,
including color.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-commit.c | 4 ++--
wt-status.c | 6 ++++++
wt-status.h | 1 +
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index ddcfffb..88b25aa 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -388,7 +388,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
wt_shortstatus_print(s, null_termination);
break;
case STATUS_FORMAT_PORCELAIN:
- wt_shortstatus_print(s, null_termination);
+ wt_porcelain_print(s, null_termination);
break;
case STATUS_FORMAT_LONG:
wt_status_print(s);
@@ -1029,7 +1029,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
wt_shortstatus_print(&s, null_termination);
break;
case STATUS_FORMAT_PORCELAIN:
- wt_shortstatus_print(&s, null_termination);
+ wt_porcelain_print(&s, null_termination);
break;
case STATUS_FORMAT_LONG:
s.verbose = verbose;
diff --git a/wt-status.c b/wt-status.c
index a8b6d05..e9bbfbc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -696,3 +696,9 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination)
wt_shortstatus_untracked(null_termination, it, s);
}
}
+
+void wt_porcelain_print(struct wt_status *s, int null_termination)
+{
+ s->use_color = 0;
+ wt_shortstatus_print(s, null_termination);
+}
diff --git a/wt-status.h b/wt-status.h
index 39c9aef..a4bddcf 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -57,5 +57,6 @@ void wt_status_print(struct wt_status *s);
void wt_status_collect(struct wt_status *s);
void wt_shortstatus_print(struct wt_status *s, int null_termination);
+void wt_porcelain_print(struct wt_status *s, int null_termination);
#endif /* STATUS_H */
--
1.6.6.rc1.292.gd8fe.dirty
^ permalink raw reply related
* [PATCHv2 1/2] t3404: Use test_commit to set up test repository
From: Michael Haggerty @ 2009-12-07 4:22 UTC (permalink / raw)
To: git; +Cc: gitster, git, Johannes.Schindelin, bgustavsson, Michael Haggerty
In-Reply-To: <cover.1260099005.git.mhagger@alum.mit.edu>
Also adjust "expected" text to reflect the file contents generated by
test_commit, which are slightly different than those generated by the
old code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
t/t3404-rebase-interactive.sh | 66 ++++++++++++----------------------------
1 files changed, 20 insertions(+), 46 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3a37793..778daf4 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -16,53 +16,26 @@ set_fake_editor
# set up two branches like this:
#
-# A - B - C - D - E
+# A - B - C - D - E (master)
# \
-# F - G - H
+# F - G - H (branch1)
# \
-# I
+# I (branch2)
#
-# where B, D and G touch the same file.
+# where A, B, D and G touch the same file.
test_expect_success 'setup' '
- : > file1 &&
- git add file1 &&
- test_tick &&
- git commit -m A &&
- git tag A &&
- echo 1 > file1 &&
- test_tick &&
- git commit -m B file1 &&
- : > file2 &&
- git add file2 &&
- test_tick &&
- git commit -m C &&
- echo 2 > file1 &&
- test_tick &&
- git commit -m D file1 &&
- : > file3 &&
- git add file3 &&
- test_tick &&
- git commit -m E &&
+ test_commit A file1 &&
+ test_commit B file1 &&
+ test_commit C file2 &&
+ test_commit D file1 &&
+ test_commit E file3 &&
git checkout -b branch1 A &&
- : > file4 &&
- git add file4 &&
- test_tick &&
- git commit -m F &&
- git tag F &&
- echo 3 > file1 &&
- test_tick &&
- git commit -m G file1 &&
- : > file5 &&
- git add file5 &&
- test_tick &&
- git commit -m H &&
+ test_commit F file4 &&
+ test_commit G file1 &&
+ test_commit H file5 &&
git checkout -b branch2 F &&
- : > file6 &&
- git add file6 &&
- test_tick &&
- git commit -m I &&
- git tag I
+ test_commit I file6
'
test_expect_success 'no changes are a nop' '
@@ -111,19 +84,20 @@ test_expect_success 'exchange two commits' '
cat > expect << EOF
diff --git a/file1 b/file1
-index e69de29..00750ed 100644
+index f70f10e..fd79235 100644
--- a/file1
+++ b/file1
-@@ -0,0 +1 @@
-+3
+@@ -1 +1 @@
+-A
++G
EOF
cat > expect2 << EOF
<<<<<<< HEAD
-2
+D
=======
-3
->>>>>>> b7ca976... G
+G
+>>>>>>> 91201e5... G
EOF
test_expect_success 'stop on conflicting pick' '
--
1.6.6.rc1.34.gd7b83
^ permalink raw reply related
* [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Michael Haggerty @ 2009-12-07 4:22 UTC (permalink / raw)
To: git; +Cc: gitster, git, Johannes.Schindelin, bgustavsson, Michael Haggerty
In-Reply-To: <cover.1260099005.git.mhagger@alum.mit.edu>
The command is like "squash", except that it discards the commit message
of the corresponding commit.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-rebase.txt | 13 ++++++----
git-rebase--interactive.sh | 51 +++++++++++++++++++++++++++++++++--------
t/lib-rebase.sh | 7 +++--
t/t3404-rebase-interactive.sh | 30 ++++++++++++++++++++++++
4 files changed, 83 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index ca5e1e8..9b648ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -382,9 +382,12 @@ If you just want to edit the commit message for a commit, replace the
command "pick" with the command "reword".
If you want to fold two or more commits into one, replace the command
-"pick" with "squash" for the second and subsequent commit. If the
-commits had different authors, it will attribute the squashed commit to
-the author of the first commit.
+"pick" for the second and subsequent commits with "squash" or "fixup".
+If the commits had different authors, the folded commit will be
+attributed to the author of the first commit. The suggested commit
+message for the folded commit is the concatenation of the commit
+messages of the first commit and of those with the "squash" command,
+but omits the commit messages of commits with the "fixup" command.
'git-rebase' will stop when "pick" has been replaced with "edit" or
when a command fails due to merge errors. When you are done editing
@@ -512,8 +515,8 @@ Easy case: The changes are literally the same.::
Hard case: The changes are not the same.::
This happens if the 'subsystem' rebase had conflicts, or used
- `\--interactive` to omit, edit, or squash commits; or if the
- upstream used one of `commit \--amend`, `reset`, or
+ `\--interactive` to omit, edit, squash, or fixup commits; or
+ if the upstream used one of `commit \--amend`, `reset`, or
`filter-branch`.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..a7de5ea 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -302,7 +302,13 @@ 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" \
+ # We want to be careful about matching only the commit
+ # message comment lines generated by this function.
+ # But supposedly some sed versions don't handle "\|"
+ # correctly, so instead of "\(st\|nd\|rd\|th\)", use
+ # the less accurate "[snrt][tdh]" to match the
+ # nth_string endings.
+ COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
< "$SQUASH_MSG" | sed -ne '$p')+1))
echo "# This is a combination of $COUNT commits."
sed -e 1d -e '2,/^./{
@@ -315,10 +321,26 @@ make_squash_message () {
echo
git cat-file commit HEAD | sed -e '1,/^$/d'
fi
- echo
- echo "# This is the $(nth_string $COUNT) commit message:"
- echo
- git cat-file commit $1 | sed -e '1,/^$/d'
+ case $1 in
+ squash)
+ echo
+ echo "# This is the $(nth_string $COUNT) commit message:"
+ echo
+ git cat-file commit $2 | sed -e '1,/^$/d'
+ ;;
+ fixup)
+ echo
+ echo "# The $(nth_string $COUNT) commit message will be skipped:"
+ echo
+ # Comment the lines of the commit message out using
+ # "#" rather than "# " (a) to make them more distinct
+ # from the explanatory comments added by this function
+ # and (b) to make it less likely that the sed regexp
+ # above will be confused by a commented-out commit
+ # message.
+ git cat-file commit $2 | sed -e '1,/^$/d' -e 's/^/#/'
+ ;;
+ esac
}
peek_next_command () {
@@ -367,20 +389,28 @@ do_next () {
warn
exit 0
;;
- squash|s)
- comment_for_reflog squash
+ squash|s|fixup|f)
+ case "$command" in
+ squash|s)
+ squash_style=squash
+ ;;
+ fixup|f)
+ squash_style=fixup
+ ;;
+ esac
+ comment_for_reflog $squash_style
test -f "$DONE" && has_action "$DONE" ||
- die "Cannot 'squash' without a previous commit"
+ die "Cannot '$squash_style' without a previous commit"
mark_action_done
- make_squash_message $sha1 > "$MSG"
+ make_squash_message $squash_style $sha1 > "$MSG"
failed=f
author_script=$(get_author_ident_from_commit HEAD)
output git reset --soft HEAD^
pick_one -n $sha1 || failed=t
case "$(peek_next_command)" in
- squash|s)
+ squash|s|fixup|f)
USE_OUTPUT=output
MSG_OPT=-F
EDIT_OR_FILE="$MSG"
@@ -768,6 +798,7 @@ first and then run 'git rebase --continue' again."
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
+# f, fixup = like "squash", but discard this commit's log message
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62f452c..f4dda02 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -9,8 +9,9 @@
#
# "[<lineno1>] [<lineno2>]..."
#
-# If a line number is prefixed with "squash", "edit", or "reword", the
-# respective line's command will be replaced with the specified one.
+# If a line number is prefixed with "squash", "fixup", "edit", or
+# "reword", the respective line's command will be replaced with the
+# specified one.
set_fake_editor () {
echo "#!$SHELL_PATH" >fake-editor.sh
@@ -32,7 +33,7 @@ cat "$1".tmp
action=pick
for line in $FAKE_LINES; do
case $line in
- squash|edit|reword)
+ squash|fixup|edit|reword)
action="$line";;
*)
echo sed -n "${line}s/^pick/$action/p"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 778daf4..ea26115 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -235,6 +235,36 @@ test_expect_success 'multi-squash only fires up editor once' '
test 1 = $(git show | grep ONCE | wc -l)
'
+test_expect_success 'multi-fixup only fires up editor once' '
+ git checkout -b multi-fixup E &&
+ base=$(git rev-parse HEAD~4) &&
+ FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \
+ git rebase -i $base &&
+ test $base = $(git rev-parse HEAD^) &&
+ test 1 = $(git show | grep ONCE | wc -l) &&
+ git checkout to-be-rebased &&
+ git branch -D multi-fixup
+'
+
+cat > expect-squash-fixup << EOF
+B
+
+D
+
+ONCE
+EOF
+
+test_expect_success 'squash and fixup generate correct log messages' '
+ git checkout -b squash-fixup E &&
+ base=$(git rev-parse HEAD~4) &&
+ FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 squash 3 fixup 4" \
+ git rebase -i $base &&
+ git cat-file commit HEAD | sed -e 1,/^\$/d > actual-squash-fixup &&
+ test_cmp expect-squash-fixup actual-squash-fixup &&
+ git checkout to-be-rebased &&
+ git branch -D squash-fixup
+'
+
test_expect_success 'squash works as expected' '
for n in one two three four
do
--
1.6.6.rc1.34.gd7b83
^ permalink raw reply related
* [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
* 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
* 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: [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 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: 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: [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: [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: [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 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: 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: 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: [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: 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox