* Re: [PATCH] pre-commit hook should ignore carriage returns at EOL
From: Christian Holtje @ 2008-06-24 19:16 UTC (permalink / raw)
To: Ian Hilt; +Cc: git
In-Reply-To: <alpine.LFD.1.10.0806241418360.32759@sys-0.hiltweb.site>
On Jun 24, 2008, at 2:26 PM, Ian Hilt wrote:
> Perhaps you want to use printf "foo\r" instead? echo "foo\r" does not
> produce a CR on my system.
...
> Wouldn't it be less redundant to do a test for \s\r$ here instead of
> testing for \r$ and then \s\r$?
The code is checking for \r$ and then doing a different space check
depending on that, not one after another.
Thanks for the feedback. I'll put up v2 in a second.
Ciao!
^ permalink raw reply
* [PATCH 3/3] git-add--interactive: manual hunk editing mode
From: Thomas Rast @ 2008-06-24 19:08 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin, Junio C Hamano
In-Reply-To: <20080624050901.GA19224@sigill.intra.peff.net>
Adds a new option 'e' to the 'add -p' command loop that lets you edit
the current hunk in your favourite editor.
If the resulting patch applies cleanly, the edited hunk will
immediately be marked for staging. If it does not apply cleanly, you
will be given an opportunity to edit again. If all lines of the hunk
are removed, then the edit is aborted and the hunk is left unchanged.
Applying the changed hunk(s) relies on Johannes Schindelin's new
--recount option for git-apply.
Note that the "real patch" test intentionally uses
(echo e; echo n; echo d) | git add -p
even though the 'n' and 'd' are superfluous at first sight. They
serve to get out of the interaction loop if git add -p wrongly
concludes the patch does not apply.
Many thanks to Jeff King <peff@peff.net> for lots of help and
suggestions.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-add.txt | 1 +
git-add--interactive.perl | 124 +++++++++++++++++++++++++++++++++++++++++++-
t/t3701-add-interactive.sh | 67 ++++++++++++++++++++++++
3 files changed, 191 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index c6de028..1d8d209 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -246,6 +246,7 @@ patch::
k - leave this hunk undecided, see previous undecided hunk
K - leave this hunk undecided, see previous hunk
s - split the current hunk into smaller hunks
+ e - manually edit the current hunk
? - print help
+
After deciding the fate for all hunks, if there is any hunk
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 903953e..6bb117a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -2,6 +2,7 @@
use strict;
use Git;
+use File::Temp;
my $repo = Git->repository();
@@ -18,6 +19,18 @@ my ($fraginfo_color) =
$diff_use_color ? (
$repo->get_color('color.diff.frag', 'cyan'),
) : ();
+my ($diff_plain_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.plain', ''),
+ ) : ();
+my ($diff_old_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.old', 'red'),
+ ) : ();
+my ($diff_new_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.new', 'green'),
+ ) : ();
my $normal_color = $repo->get_color("", "reset");
@@ -770,6 +783,104 @@ sub coalesce_overlapping_hunks {
return @out;
}
+sub color_diff {
+ return map {
+ colored((/^@/ ? $fraginfo_color :
+ /^\+/ ? $diff_new_color :
+ /^-/ ? $diff_old_color :
+ $diff_plain_color),
+ $_);
+ } @_;
+}
+
+sub edit_hunk_manually {
+ my ($oldtext) = @_;
+
+ my $t = File::Temp->new(
+ TEMPLATE => $repo->repo_path . "/git-hunk-edit.XXXXXX",
+ SUFFIX => '.diff'
+ );
+ print $t "# Manual hunk edit mode -- see bottom for a quick guide\n";
+ print $t @$oldtext;
+ print $t <<EOF;
+# ---
+# To remove '-' lines, make them ' ' lines (context).
+# To remove '+' lines, delete them.
+# Lines starting with # will be removed.
+#
+# If the patch applies cleanly, the edited hunk will immediately be
+# marked for staging. If it does not apply cleanly, you will be given
+# an opportunity to edit again. If all lines of the hunk are removed,
+# then the edit is aborted and the hunk is left unchanged.
+EOF
+ close $t;
+
+ my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor")
+ || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+ system('sh', '-c', $editor.' "$@"', $editor, $t);
+
+ open my $fh, '<', $t
+ or die "failed to open hunk edit file for reading: " . $!;
+ my @newtext = grep { !/^#/ } <$fh>;
+ close $fh;
+
+ # Abort if nothing remains
+ if (!grep { /\S/ } @newtext) {
+ return undef;
+ }
+
+ # Reinsert the first hunk header if the user accidentally deleted it
+ if ($newtext[0] !~ /^@/) {
+ unshift @newtext, $oldtext->[0];
+ }
+ return \@newtext;
+}
+
+sub diff_applies {
+ my $fh;
+ open $fh, '| git apply --recount --cached --check';
+ for my $h (@_) {
+ print $fh @{$h->{TEXT}};
+ }
+ return close $fh;
+}
+
+sub prompt_yesno {
+ my ($prompt) = @_;
+ while (1) {
+ print colored $prompt_color, $prompt;
+ my $line = <STDIN>;
+ return 0 if $line =~ /^n/i;
+ return 1 if $line =~ /^y/i;
+ }
+}
+
+sub edit_hunk_loop {
+ my ($head, $hunk, $ix) = @_;
+ my $text = $hunk->[$ix]->{TEXT};
+
+ while (1) {
+ $text = edit_hunk_manually($text);
+ if (!defined $text) {
+ return undef;
+ }
+ my $newhunk = { TEXT => $text, USE => 1 };
+ if (diff_applies($head,
+ @{$hunk}[0..$ix-1],
+ $newhunk,
+ @{$hunk}[$ix+1..$#{$hunk}])) {
+ $newhunk->{DISPLAY} = [color_diff(@{$text})];
+ return $newhunk;
+ }
+ else {
+ prompt_yesno(
+ 'Your edited hunk does not apply. Edit again '
+ . '(saying "no" discards!) [y/n]? '
+ ) or return undef;
+ }
+ }
+}
+
sub help_patch_cmd {
print colored $help_color, <<\EOF ;
y - stage this hunk
@@ -781,6 +892,7 @@ J - leave this hunk undecided, see next hunk
k - leave this hunk undecided, see previous undecided hunk
K - leave this hunk undecided, see previous hunk
s - split the current hunk into smaller hunks
+e - manually edit the current hunk
? - print help
EOF
}
@@ -846,6 +958,7 @@ sub patch_update_file {
$num = scalar @hunk;
$ix = 0;
+ my $need_recount = 0;
while (1) {
my ($prev, $next, $other, $undecided, $i);
@@ -885,6 +998,7 @@ sub patch_update_file {
if (hunk_splittable($hunk[$ix]{TEXT})) {
$other .= '/s';
}
+ $other .= '/e';
for (@{$hunk[$ix]{DISPLAY}}) {
print;
}
@@ -949,6 +1063,13 @@ sub patch_update_file {
$num = scalar @hunk;
next;
}
+ elsif ($line =~ /^e/) {
+ my $newhunk = edit_hunk_loop($head, \@hunk, $ix);
+ if (defined $newhunk) {
+ splice @hunk, $ix, 1, $newhunk;
+ $need_recount = 1;
+ }
+ }
else {
help_patch_cmd($other);
next;
@@ -1002,7 +1123,8 @@ sub patch_update_file {
if (@result) {
my $fh;
- open $fh, '| git apply --cached';
+ open $fh, '| git apply --cached'
+ . ($need_recount ? ' --recount' : '');
for (@{$head->{TEXT}}, @result) {
print $fh $_;
}
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index fae64ea..e95663d 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -66,6 +66,73 @@ test_expect_success 'revert works (commit)' '
grep "unchanged *+3/-0 file" output
'
+cat >expected <<EOF
+EOF
+cat >fake_editor.sh <<EOF
+EOF
+chmod a+x fake_editor.sh
+test_set_editor "$(pwd)/fake_editor.sh"
+test_expect_success 'dummy edit works' '
+ (echo e; echo a) | git add -p &&
+ git diff > diff &&
+ test_cmp expected diff
+'
+
+cat >patch <<EOF
+@@ -1,1 +1,4 @@
+ this
++patch
+-doesn't
+ apply
+EOF
+echo "#!$SHELL_PATH" >fake_editor.sh
+cat >>fake_editor.sh <<\EOF
+mv -f "$1" oldpatch &&
+mv -f patch "$1"
+EOF
+chmod a+x fake_editor.sh
+test_set_editor "$(pwd)/fake_editor.sh"
+test_expect_success 'bad edit rejected' '
+ git reset &&
+ (echo e; echo n; echo d) | git add -p >output &&
+ grep "hunk does not apply" output
+'
+
+cat >patch <<EOF
+this patch
+is garbage
+EOF
+test_expect_success 'garbage edit rejected' '
+ git reset &&
+ (echo e; echo n; echo d) | git add -p >output &&
+ grep "hunk does not apply" output
+'
+
+cat >patch <<EOF
+@@ -1,0 +1,0 @@
+ baseline
++content
++newcontent
++lines
+EOF
+cat >expected <<EOF
+diff --git a/file b/file
+index b5dd6c9..f910ae9 100644
+--- a/file
++++ b/file
+@@ -1,4 +1,4 @@
+ baseline
+ content
+-newcontent
++more
+ lines
+EOF
+test_expect_success 'real edit works' '
+ (echo e; echo n; echo d) | git add -p &&
+ git diff >output &&
+ test_cmp expected output
+'
+
if test "$(git config --bool core.filemode)" = false
then
say 'skipping filemode tests (filesystem does not properly support modes)'
--
1.5.6.84.ge5c1
^ permalink raw reply related
* [PATCH 2/3] git-add: introduce --edit (to edit the diff vs. the index)
From: Thomas Rast @ 2008-06-24 19:08 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin, Junio C Hamano
In-Reply-To: <20080624050901.GA19224@sigill.intra.peff.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
With "git add -e [<files>]", Git will fire up an editor with the current
diff relative to the index (i.e. what you would get with "git diff
[<files>]").
Now you can edit the patch as much as you like, including adding/removing
lines, editing the text, whatever. Make sure, though, that the first
character of the hunk lines is still a space, a plus or a minus.
After you closed the editor, Git will adjust the line counts of the
hunks if necessary, thanks to the --fixup-line-counts option of apply,
and commit the patch. Except if you deleted everything, in which case
nothing happens (for obvious reasons).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-add.txt | 12 ++++-
builtin-add.c | 55 +++++++++++++++++++-
t/t3702-add-edit.sh | 126 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 191 insertions(+), 2 deletions(-)
create mode 100755 t/t3702-add-edit.sh
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index b8e3fa6..c6de028 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git-add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
- [--update | -u] [--refresh] [--ignore-errors] [--]
+ [--edit | -e] [--update | -u] [--refresh] [--ignore-errors] [--]
<filepattern>...
DESCRIPTION
@@ -76,6 +76,16 @@ OPTIONS
bypassed and the 'patch' subcommand is invoked using each of
the specified filepatterns before exiting.
+-e::
+--edit::
+ Open the diff vs. the index in an editor and let the user
+ edit it. After the editor was closed, adjust the hunk headers
+ and apply the patch to the index.
++
+*NOTE*: Obviously, if you change anything else than the first character
+on lines beginning with a space or a minus, the patch will no longer
+apply.
+
-u::
--update::
Update only files that git already knows about, staging modified
diff --git a/builtin-add.c b/builtin-add.c
index 9930cf5..b9e9a17 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,7 +19,7 @@ static const char * const builtin_add_usage[] = {
"git-add [options] [--] <filepattern>...",
NULL
};
-static int patch_interactive = 0, add_interactive = 0;
+static int patch_interactive = 0, add_interactive = 0, edit_interactive = 0;
static int take_worktree_changes;
static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
@@ -186,6 +186,56 @@ int interactive_add(int argc, const char **argv, const char *prefix)
return status;
}
+int edit_patch(int argc, const char **argv, const char *prefix)
+{
+ char *file = xstrdup(git_path("ADD_EDIT.patch"));
+ const char *apply_argv[] = { "apply", "--recount", "--cached",
+ file, NULL };
+ struct child_process child;
+ int result = 0, ac;
+ struct stat st;
+
+ memset(&child, 0, sizeof(child));
+ child.argv = xcalloc(sizeof(const char *), (argc + 5));
+ ac = 0;
+ child.git_cmd = 1;
+ child.argv[ac++] = "diff-files";
+ child.argv[ac++] = "--no-color";
+ child.argv[ac++] = "-p";
+ child.argv[ac++] = "--";
+ if (argc) {
+ const char **pathspec = validate_pathspec(argc, argv, prefix);
+ if (!pathspec)
+ return -1;
+ memcpy(&(child.argv[ac]), pathspec, sizeof(*argv) * argc);
+ ac += argc;
+ }
+ child.argv[ac] = NULL;
+ child.out = open(file, O_CREAT | O_WRONLY, 0644);
+ result = child.out < 0 && error("Could not write to '%s'", file);
+
+ if (!result)
+ result = run_command(&child);
+ free(child.argv);
+
+ launch_editor(file, NULL, NULL);
+
+ if (!result)
+ result = stat(file, &st) && error("Could not stat '%s'", file);
+ if (!result && !st.st_size)
+ result = error("Empty patch. Aborted.");
+
+ memset(&child, 0, sizeof(child));
+ child.git_cmd = 1;
+ child.argv = apply_argv;
+ if (!result)
+ result = run_command(&child) &&
+ error("Could not apply '%s'", file);
+ if (!result)
+ unlink(file);
+ return result;
+}
+
static struct lock_file lock_file;
static const char ignore_error[] =
@@ -202,6 +252,7 @@ static struct option builtin_add_options[] = {
OPT_BOOLEAN('p', "patch", &patch_interactive, "interactive patching"),
OPT_BOOLEAN('f', "force", &ignored_too, "allow adding otherwise ignored files"),
OPT_BOOLEAN('u', "update", &take_worktree_changes, "update tracked files"),
+ OPT_BOOLEAN('e', "edit", &edit_interactive, "super-interactive patching"),
OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
OPT_END(),
@@ -226,6 +277,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, builtin_add_options,
builtin_add_usage, 0);
+ if (edit_interactive)
+ return(edit_patch(argc, argv, prefix));
if (patch_interactive)
add_interactive = 1;
if (add_interactive)
diff --git a/t/t3702-add-edit.sh b/t/t3702-add-edit.sh
new file mode 100755
index 0000000..decf727
--- /dev/null
+++ b/t/t3702-add-edit.sh
@@ -0,0 +1,126 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='add -e basic tests'
+. ./test-lib.sh
+
+
+cat > file << EOF
+LO, praise of the prowess of people-kings
+of spear-armed Danes, in days long sped,
+we have heard, and what honor the athelings won!
+Oft Scyld the Scefing from squadroned foes,
+from many a tribe, the mead-bench tore,
+awing the earls. Since erst he lay
+friendless, a foundling, fate repaid him:
+for he waxed under welkin, in wealth he throve,
+till before him the folk, both far and near,
+who house by the whale-path, heard his mandate,
+gave him gifts: a good king he!
+EOF
+
+test_expect_success 'setup' '
+
+ git add file &&
+ test_tick &&
+ git commit -m initial file
+
+'
+
+cat > patch << EOF
+diff --git a/file b/file
+index b9834b5..ef6e94c 100644
+--- a/file
++++ b/file
+@@ -3,1 +3,333 @@ of spear-armed Danes, in days long sped,
+ we have heard, and what honor the athelings won!
++
+ Oft Scyld the Scefing from squadroned foes,
+@@ -2,7 +1,5 @@ awing the earls. Since erst he lay
+ friendless, a foundling, fate repaid him:
++
+ for he waxed under welkin, in wealth he throve,
+EOF
+
+cat > expected << EOF
+diff --git a/file b/file
+index b9834b5..ef6e94c 100644
+--- a/file
++++ b/file
+@@ -1,10 +1,12 @@
+ LO, praise of the prowess of people-kings
+ of spear-armed Danes, in days long sped,
+ we have heard, and what honor the athelings won!
++
+ Oft Scyld the Scefing from squadroned foes,
+ from many a tribe, the mead-bench tore,
+ awing the earls. Since erst he lay
+ friendless, a foundling, fate repaid him:
++
+ for he waxed under welkin, in wealth he throve,
+ till before him the folk, both far and near,
+ who house by the whale-path, heard his mandate,
+EOF
+
+echo "#!$SHELL_PATH" >fake-editor.sh
+cat >> fake-editor.sh <<\EOF
+mv -f "$1" orig-patch &&
+mv -f patch "$1"
+EOF
+
+test_set_editor "$(pwd)/fake-editor.sh"
+chmod a+x fake-editor.sh
+
+test_expect_success 'add -e' '
+
+ cp fake-editor.sh file &&
+ git add -e &&
+ test_cmp fake-editor.sh file &&
+ git diff --cached > out &&
+ test_cmp out expected
+
+'
+
+cat > patch << EOF
+diff --git a/file b/file
+--- a/file
++++ b/file
+@@ -1,1 +1,1 @@
+ gave him gifts: a good king he!
++
+EOF
+
+test_expect_success 'add -e adds to the end of the file' '
+
+ test_tick &&
+ git commit -m update &&
+ git checkout &&
+ git add -e &&
+ git diff --cached > out &&
+ test "" = "$(git show :file | tail -n 1)"
+
+'
+
+cat > patch << EOF
+diff --git a/file b/file
+--- a/file
++++ b/file
+@@ -1,1 +1,1 @@
++
+ LO, praise of the prowess of people-kings
+EOF
+
+test_expect_success 'add -e adds to the beginning of the file' '
+
+ test_tick &&
+ git commit -m update &&
+ git checkout &&
+ git add -e &&
+ git diff --cached > out &&
+ test "" = "$(git show :file | head -n 1)"
+
+'
+
+test_done
--
1.5.6.84.ge5c1
^ permalink raw reply related
* [PATCH 0/3] Manual editing for 'add' and 'add -p'
From: Thomas Rast @ 2008-06-24 19:07 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin, Junio C Hamano
In-Reply-To: <20080624050901.GA19224@sigill.intra.peff.net>
Jeff King wrote:
> Thomas, do you want to just re-submit the "--recount" patches when you
> re-submit your patch?
If that is the right thing to do :-)
I rebased Johannes' patches to current 'next', which had some trivial
merge conflicts, and made one actual change: By analogy with one
earlier patch that caused some of the conflicts, I split '-e' and
'--edit' across two lines in the documentation.
I also integrated your last suggestion:
> # Abort if nothing remains
> if (!grep { /\S/ } @newtext) {
> return undef;
> }
I'm not really convinced it is needed, but any such patch can
obviously not change anything, so it can't hurt. Who knows, perhaps
there are editors brain-damaged enough to always save at least one
newline. (This is the only change to my patch compared to v4.)
Johannes Schindelin wrote:
>
> To spare you following that link: Junio wanted to reuse "git apply
> --recount" to apply mboxes, where a separator "^-- $" to the signature is
> quite common, and could be mistaken for a "-" line of a hunk.
[...]
> However, I think that this issue should not concern us _now_. As long as
> --recount is only to be used in "add -i" and "add -e", I think the patch
> is good as is:
I'm wondering if this should be turned into docs. After all, it's not
some DWIM option. It just says that instead of trusting the counts,
lines starting with "diff " or "@@ " start hunks, implying that all
/^[-+ ]/ lines in between are significant.
I hope this series turns out right in mail; I'm struggling a bit to
properly import the mails to KMail. Most annoyingly, format-patch
apparently cannot turn other people's patches into mails by myself
with an appropriate "From:" line. :-(
- Thomas
Johannes Schindelin (2):
Allow git-apply to ignore the hunk headers (AKA recountdiff)
git-add: introduce --edit (to edit the diff vs. the index)
Thomas Rast (1):
git-add--interactive: manual hunk editing mode
Documentation/git-add.txt | 13 ++++-
Documentation/git-apply.txt | 7 ++-
builtin-add.c | 55 ++++++++++++++++++-
builtin-apply.c | 64 ++++++++++++++++++++--
git-add--interactive.perl | 124 +++++++++++++++++++++++++++++++++++++++++-
t/t3701-add-interactive.sh | 67 +++++++++++++++++++++++
t/t3702-add-edit.sh | 126 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 448 insertions(+), 8 deletions(-)
create mode 100755 t/t3702-add-edit.sh
^ permalink raw reply
* [PATCH 1/3] Allow git-apply to ignore the hunk headers (AKA recountdiff)
From: Thomas Rast @ 2008-06-24 19:08 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin, Junio C Hamano
In-Reply-To: <20080624050901.GA19224@sigill.intra.peff.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Sometimes, the easiest way to fix up a patch is to edit it directly, even
adding or deleting lines. Now, many people are not as divine as certain
benevolent dictators as to update the hunk headers correctly at the first
try.
So teach the tool to do it for us.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-apply.txt | 7 ++++-
builtin-apply.c | 64 ++++++++++++++++++++++++++++++++++++++++---
2 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index c834763..b8ab6ed 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -12,7 +12,7 @@ SYNOPSIS
'git-apply' [--stat] [--numstat] [--summary] [--check] [--index]
[--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
[--allow-binary-replacement | --binary] [--reject] [-z]
- [-pNUM] [-CNUM] [--inaccurate-eof] [--cached]
+ [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
[--whitespace=<nowarn|warn|fix|error|error-all>]
[--exclude=PATH] [--verbose] [<patch>...]
@@ -171,6 +171,11 @@ behavior:
correctly. This option adds support for applying such patches by
working around this bug.
+--recount::
+ Do not trust the line counts in the hunk headers, but infer them
+ by inspecting the patch (e.g. after editing the patch without
+ adjusting the hunk headers appropriately).
+
-v::
--verbose::
Report progress to stderr. By default, only a message about the
diff --git a/builtin-apply.c b/builtin-apply.c
index c497889..34c220f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -153,6 +153,7 @@ struct patch {
unsigned int is_binary:1;
unsigned int is_copy:1;
unsigned int is_rename:1;
+ unsigned int recount:1;
struct fragment *fragments;
char *result;
size_t resultsize;
@@ -882,6 +883,50 @@ static int parse_range(const char *line, int len, int offset, const char *expect
return offset + ex;
}
+static int recount_diff(char *line, int size, struct fragment *fragment)
+{
+ int line_nr = 0;
+
+ if (size < 1)
+ return -1;
+
+ fragment->oldpos = 2;
+ fragment->oldlines = fragment->newlines = 0;
+
+ for (;;) {
+ int len = linelen(line, size);
+ size -= len;
+ line += len;
+
+ if (size < 1)
+ return 0;
+
+ switch (*line) {
+ case ' ': case '\n':
+ fragment->newlines++;
+ /* fall through */
+ case '-':
+ fragment->oldlines++;
+ break;
+ case '+':
+ fragment->newlines++;
+ if (line_nr == 0) {
+ fragment->leading = 1;
+ fragment->oldpos = 1;
+ }
+ fragment->trailing = 1;
+ break;
+ case '@':
+ return size < 3 || prefixcmp(line, "@@ ");
+ case 'd':
+ return size < 5 || prefixcmp(line, "diff ");
+ default:
+ return -1;
+ }
+ line_nr++;
+ }
+}
+
/*
* Parse a unified diff fragment header of the
* form "@@ -a,b +c,d @@"
@@ -1013,6 +1058,9 @@ static int parse_fragment(char *line, unsigned long size,
offset = parse_fragment_header(line, len, fragment);
if (offset < 0)
return -1;
+ if (offset > 0 && patch->recount &&
+ recount_diff(line + offset, size - offset, fragment))
+ return -1;
oldlines = fragment->oldlines;
newlines = fragment->newlines;
leading = 0;
@@ -2912,7 +2960,8 @@ static void prefix_patches(struct patch *p)
}
}
-static int apply_patch(int fd, const char *filename, int inaccurate_eof)
+static int apply_patch(int fd, const char *filename, int inaccurate_eof,
+ int recount)
{
size_t offset;
struct strbuf buf;
@@ -2929,6 +2978,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)
patch = xcalloc(1, sizeof(*patch));
patch->inaccurate_eof = inaccurate_eof;
+ patch->recount = recount;
nr = parse_chunk(buf.buf + offset, buf.len - offset, patch);
if (nr < 0)
break;
@@ -2998,6 +3048,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
int i;
int read_stdin = 1;
int inaccurate_eof = 0;
+ int recount = 0;
int errs = 0;
int is_not_gitdir;
@@ -3015,7 +3066,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
int fd;
if (!strcmp(arg, "-")) {
- errs |= apply_patch(0, "<stdin>", inaccurate_eof);
+ errs |= apply_patch(0, "<stdin>", inaccurate_eof,
+ recount);
read_stdin = 0;
continue;
}
@@ -3118,6 +3170,10 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
inaccurate_eof = 1;
continue;
}
+ if (!strcmp(arg, "--recount")) {
+ recount = 1;
+ continue;
+ }
if (0 < prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
@@ -3126,12 +3182,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
die("can't open patch '%s': %s", arg, strerror(errno));
read_stdin = 0;
set_default_whitespace_mode(whitespace_option);
- errs |= apply_patch(fd, arg, inaccurate_eof);
+ errs |= apply_patch(fd, arg, inaccurate_eof, recount);
close(fd);
}
set_default_whitespace_mode(whitespace_option);
if (read_stdin)
- errs |= apply_patch(0, "<stdin>", inaccurate_eof);
+ errs |= apply_patch(0, "<stdin>", inaccurate_eof, recount);
if (whitespace_error) {
if (squelch_whitespace_errors &&
squelch_whitespace_errors < whitespace_error) {
--
1.5.6.84.ge5c1
^ permalink raw reply related
* Re: What's cooking in git.git (topics)
From: Johannes Schindelin @ 2008-06-24 19:08 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Pieter de Bie, Junio C Hamano, git
In-Reply-To: <20080624185403.GB29404@genesis.frugalware.org>
Hi,
On Tue, 24 Jun 2008, Miklos Vajna wrote:
> On Tue, Jun 24, 2008 at 05:25:57PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > Vienna:bin pieter$ git --version
> > > > git version 1.5.6.129.g274ea
> > > > Vienna:bin pieter$ git clone localhost:project/bonnenteller
> > > > Initialize bonnenteller/.git
> > > > Initialized empty Git repository in /opt/git/bin/bonnenteller/.git/
> > > > Password:
> > > > bash: git-upload-pack: command not found
> > > > fatal: The remote end hung up unexpectedly
> > > >
> > > > I think that is what Miklos meant.
> > >
> > > Exactly. Thanks for the good description.
> >
> > AFAICT these are fixed with ed99a225(Merge branch 'jc/dashless' into
> > next).
>
> Using fc48199 ("Merge branch 'master' into next", which includes
> ed99a225) on the server, v1.5.6 on the client, I get:
>
> $ git clone server:/home/vmiklos/git/test next
> Initialize next/.git
> Initialized empty Git repository in /home/vmiklos/scm/git/next/.git/
> vmiklos@server's password:
> bash: git-upload-pack: command not found
> fatal: The remote end hung up unexpectedly
Hmm. Probably the client needs to be newer, too. This is going to be
painful. Junio?
Sorry for the noise,
Dscho
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: Jakub Narebski @ 2008-06-24 19:08 UTC (permalink / raw)
To: Boaz Harrosh; +Cc: David Jeske, git
In-Reply-To: <48612CB0.2070303@panasas.com>
Boaz Harrosh <bharrosh@panasas.com> writes:
> Sorry
> git-reset --clean -f/-n for removing local changes
> git reset --hard for moving HEAD on a clean tree only
Wouldn't "git reset <commit-ish>" be enough then? It modifies where
current branch points to (as opposed to git-checkout modifying what is
the current branch), and it modifies index. What it doesn't modify is
working directory, but it is clean already.
So the solution is: don't use `--hard'.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] pre-commit hook should ignore carriage returns at EOL
From: Jakub Narebski @ 2008-06-24 19:05 UTC (permalink / raw)
To: Ian Hilt; +Cc: Christian Holtje, git, gitster
In-Reply-To: <alpine.LFD.1.10.0806241418360.32759@sys-0.hiltweb.site>
Ian Hilt <Ian.Hilt@gmx.com> writes:
> On Tue, 24 Jun 2008 at 12:23pm -0400, Christian Holtje wrote:
> > --- a/templates/hooks--pre-commit
> > +++ b/templates/hooks--pre-commit
> > @@ -55,8 +55,14 @@ perl -e '
> > if (s/^\+//) {
> > $lineno++;
> > chomp;
> > - if (/\s$/) {
> > - bad_line("trailing whitespace", $_);
> > + if (/\r$/) {
>
> Wouldn't it be less redundant to do a test for \s\r$ here instead of
> testing for \r$ and then \s\r$?
I don't think so, because you want next to test for whitespace
where it _doesn't_ end in \r, i.e. this condition is here
because of the 'else' clause. IIRC.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: git-clone works with ssh but not with http/https/git
From: Jakub Narebski @ 2008-06-24 18:58 UTC (permalink / raw)
To: Anton Gladkov
Cc: Erez Zilber, Johannes Schindelin, Matthias Kestenholz,
git@vger.kernel.org
In-Reply-To: <20080624135429.GA6905@atn.sw.ru>
On Tue, 24 Jun 2008, Anton Gladkov wrote:
> On Tue, Jun 24, 2008 at 05:42:14PM +0400, Erez Zilber wrote:
> >
> > I guess that the problem is that no proper mapping exists. That's why
> > I see the following in /var/log/httpd/error_log:
> >
> > [Tue Jun 24 16:31:52 2008] [error] [client 172.16.0.7] File does not
> > exist: /var/www/html/pub
> >
> > What do I need to add in /etc/httpd/conf.d/ in order to set the
> > mapping to /pub/git instead of /var/www/html/pub ? Is there an example
> > that shows how to map?
>
> IMO the simplest way is to create a symlink 'pub' in /var/www/html directory
> pointing to /pub and to add 'Options FollowSymLinks' to <Directory /> in httpd.conf.
Another solution (not necessary simplest, but it might be preferred
from the security point of view) is to do some URL rewriting / redirection.
See your web server documentation for details...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Segmentation fault on http clone, post-1.5.6
From: Teemu Likonen @ 2008-06-24 18:57 UTC (permalink / raw)
To: Jeff King; +Cc: git, Nicolas Pitre
In-Reply-To: <20080624164034.GB4654@sigill.intra.peff.net>
Jeff King wrote (2008-06-24 12:40 -0400):
> On Tue, Jun 24, 2008 at 04:04:57PM +0300, Teemu Likonen wrote:
>
> > With the current "master" branch version (29b0d0191) I get
> > segmentation fault when trying to clone a git repo with http
> > protocol. Tried a couple of times and it's always reproducible. You
> > can test with the following repository (about 5.5 MB):
> >
> > git clone http://www.iki.fi/tlikonen/voikko.git
>
> I can't reproduce the segfault here.
>
> > I also build git from the tag v1.5.6 and it seems to work fine, so
> > I guess the bug was introduced after 1.5.6.
>
> That sounds like an excellent opportunity to learn about git-bisect.
> Can you try bisecting the bug and reporting back the problematic
> commit?
Indeed. I have now officially bisected the problem and the first bad or
problematic commit is 8eca0b47 "implement some resilience against pack
corruptions" (hence Cc to Nicolas, the author). This is always
reproducible in my Debian 4.0 box.
^ permalink raw reply
* Re: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.
From: Jakub Narebski @ 2008-06-24 18:55 UTC (permalink / raw)
To: Adam Brewster; +Cc: git
In-Reply-To: <c376da900806240830p2a48aff0uaf6f22372fead5ef@mail.gmail.com>
Adam Brewster wrote:
>> [...]
>>> Then you can add the objects in the bundle to the basis, so they won't
>>> get included in the next pack like this:
>>>
>>> $ git-basis --update my-basis < my-bundle
>>
>> Why not use "$(git ls-remote my-bundle)" somewhere in the invocation
>> creating new bundle instead?
>>
>
> You could use "git ls-remote my-bundle | git-basis --update my-basis"
> to do the same thing as the command I gave above.
I was thinking about
$ git bundle create my-bundle --all --not $(git ls-remote my-bundle | cut -f1)
(or `--branches' instead of `--all'). Or, if you don't want to keep
bundle, but only save basis, just use
$ git bundle create my-bundle --all --not $(git basis --show my-basis)
No need for `--stdin' / `--revs' / `--basis' option to git-bundle,
at the cost of little shell trickery.
Or even
$ git ls-remote my-bundle | cut -f1 > my-bases
[...]
$ git bundle create my-bundle --all --not $(cat my-bases)
>>> I'm sure that my implementation is crap, but I think this is a useful
>>> idea. Anybody agree? Disagree?
>>
>> Documentation, please? Especially that it looks like '--stdin' option
>> is a bit tricky...
>
> I wanted to test the waters and make sure that someone was at least
> vaguely interested in this (no need to document code that is never
> going to leave my machine).
See above example, to check if this would be enough...
> I'll prepare another patch with documentation and changing --stdin to
> --revs when I get a chance.
I'm not sure if another name, like --bases=<filename / basename> wouldn't
be better. Perhaps --stdin is a good name...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: Brandon Casey @ 2008-06-24 18:55 UTC (permalink / raw)
To: David Jeske; +Cc: git
In-Reply-To: <willow-jeske-01l5xqJDFEDjCftd>
David Jeske wrote:
> My takeaways from this thread:
>
> - THANKS! to all of you for the detailed discussion, and for making git. Even
> though it's still unfamiliar to me, I really enjoy (g)it!
>
> - I don't think anyone here thinks git is beyond improvement. This discussion
> did change my mind on a few things since my original post. I started this
> discussion to share my "unacclimated usability suggestions", because after I
> acclimate to git, I'll be telling new users that these idiosyncrasies are all
> no big deal too. :) I still think there is value in this list of suggestions.
> I'll work on submitting patches...
>
> - improve the man page description of "reset --hard" (see below)
> - standardize all the potentially destructive operations (after gc) on "-f /
> --force" to override
The thing is 'force' is not always the most descriptive word for the behavior
that you propose enabling with --force.
For the reset command in particular there is a --soft counterpart to --hard. They
are both modifiers on the term 'reset' i.e. a 'soft reset' or a 'hard reset'. The
default is wbat is called a 'mixed reset'.
'gc' is another command that has been mentioned along with its '--aggressive' option.
--force does not seem to make sense here either, since we are not necessarily forcing
anything to happen in the sense of overriding some safe guard. What is happening is
that possibly more cpu-intensive options are being selected when repacking (compressing)
the repository.
> Consider branch which has
> both "branch -[MD]" and "branch -f" in the same subcommand. What's wrong with
> "branch -[md] -f"?
I am inclined to agree here. I'm not sure why the options for 'git branch' were
created this way. I too have thought that a -f modifier on -m and -d would be
more intuitive.
> Of course --hard encourages one to read the manpage. However, git is using a
> bunch of new terms for things, and uses at least those three different methods
> to indicate command danger. Lets look at the working on the manpage:
>
> "Matches the working tree and index to that of the tree being
> switched
> to. Any changes to tracked files in the working tree since <commit>
> are lost."
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> I interpreted this as "any [non committed changes] to tracked files in the
> working tree since <commit> are lost." I don't this this was a naive
> interpretation. I still think that's the way it reads after this whole
> conversation.
I think the reason it only says that uncommitted changes are lost is because
the committed changes are not lost even though they may become unreachable
from the head of the current branch. They are still reachable at least from
the reflog, so they are not lost. The uncommitted changes _are_ lost and are
unrecoverable.
> I'll work on my first patch for git:
>
> -> "References to any working tree changes, and pulled changes, AND COMMITTED
> CHANGES to tracked files in the branch after <commit> will be dropped, causing
> them to be removed at the next garbage collect.".
Uncommited working tree changes are gone immediately. Anything that has already
been committed will be garbage collected only after it is not referenced by
anything else in the repository. A reference will be maintained in the reflog
for at least 30 days (by default).
>
> -- Brandon Casey wrote:
>> After saying all of that, here is how I think you _should_ have done things.
>> Notice I _did_not_ use 'reset --hard'.
>
> I was told that I can safely do "git checkout origin/master" instead of "reset
> --hard" to get back to the pull point, in case I didn't branch ahead of time.
I think 'git checkout origin/master' would be a little odd since this is usually
a remote tracking branch. 'git checkout -b mymaster origin/master' or similar
would be more common. This creates a new branch named 'mymaster'.
-brandon
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Miklos Vajna @ 2008-06-24 18:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pieter de Bie, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0806241709330.9925@racer>
[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]
On Tue, Jun 24, 2008 at 05:25:57PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > Vienna:bin pieter$ git --version
> > > git version 1.5.6.129.g274ea
> > > Vienna:bin pieter$ git clone localhost:project/bonnenteller
> > > Initialize bonnenteller/.git
> > > Initialized empty Git repository in /opt/git/bin/bonnenteller/.git/
> > > Password:
> > > bash: git-upload-pack: command not found
> > > fatal: The remote end hung up unexpectedly
> > >
> > > I think that is what Miklos meant.
> >
> > Exactly. Thanks for the good description.
>
> AFAICT these are fixed with ed99a225(Merge branch 'jc/dashless' into
> next).
Using fc48199 ("Merge branch 'master' into next", which includes
ed99a225) on the server, v1.5.6 on the client, I get:
$ git clone server:/home/vmiklos/git/test next
Initialize next/.git
Initialized empty Git repository in /home/vmiklos/scm/git/next/.git/
vmiklos@server's password:
bash: git-upload-pack: command not found
fatal: The remote end hung up unexpectedly
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* git svn clone a non-standard repository
From: John Locke @ 2008-06-24 18:32 UTC (permalink / raw)
To: git
Hi,
I'm trying to create a git repo of the Dojo Toolkit, which has a quite
non-standard layout. There are 4 modules to the current Dojo project
that I care about (and some others in the repo that I'm not interested
in). The problem is, the trunk code is split out into
<modulename>/trunk, while tags are in tags/<tagname>/<modulename> and
branches are in branches/<branchname>/<modulename>. e.g:
moduleA/trunk <- contains trunk development of moduleA
moduleA/tags <- empty
moduleA/branches <- empty
moduleB/trunk
moduleB/tags
moduleB/branches
moduleC/trunk
moduleC/tags
moduleC/branches
moduleD/trunk
moduleD/tags
moduleD/branches
moduleE/trunk <- I don't care about this one...
trunk/ <- contains ancient version, not actual trunk
tags/1.0.0/moduleA <- contains tagged version of moduleA
tags/1.0.0/moduleB <- contains tagged version of moduleB
tags/1.0.0/moduleC <- contains tagged version of moduleC
tags/1.0.0/moduleD <- contains tagged version of moduleD
tags/1.0.1/moduleA
tags/1.0.1/moduleB
...
So I'd like to set up a git repo that tracks this SVN repository, and
allows me to see:
moduleA/
moduleB/
moduleC/
moduleD/
... in my checkout, whether I'm on trunk or a tag.
What's the best way to set this up?
I've started with "git svn clone http://path/to/svn -T moduleA/trunk -t
tags -b branches", and it's been sucking down branches for a couple days
now, still not done. Can I set up moduleB/moduleC/moduleD as additional
remotes in this same repository, and end up with the desired result? Was
thinking I would add additional svn sections to .git/config, and then
git svn fetch -- will this work, or is there a better way?
Thanks,
--
John Locke
"Open Source Solutions for Small Business Problems"
published by Charles River Media, June 2004
http://www.freelock.com
^ permalink raw reply
* Re: [PATCH] pre-commit hook should ignore carriage returns at EOL
From: Ian Hilt @ 2008-06-24 18:26 UTC (permalink / raw)
To: Christian Holtje; +Cc: git, gitster
In-Reply-To: <53A5AFCF-94C7-465E-A181-1DA69F251F5B@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3387 bytes --]
On Tue, 24 Jun 2008 at 12:23pm -0400, Christian Holtje wrote:
> When commit files that use DOS style CRLF end-of-lines, the pre-commit
> hook would raise an error. When combined with the fact that the hooks
> get activated by default on windows, it makes life difficult for
> people working with DOS files.
>
> This patch causes the pre-commit hook to deal with crlf files
> correctly.
>
> Signed-off-by: Christian Höltje <docwhat@gmail.com>
> ---
> t/t7503-template-hook--pre-commit.sh | 75 ++++++++++++++++++++++++++++++++++
> templates/hooks--pre-commit | 10 ++++-
> 2 files changed, 83 insertions(+), 2 deletions(-)
> create mode 100755 t/t7503-template-hook--pre-commit.sh
>
> diff --git a/t/t7503-template-hook--pre-commit.sh
> b/t/t7503-template-hook--pre-commit.sh
> new file mode 100755
> index 0000000..8f0c3c9
> --- /dev/null
> +++ b/t/t7503-template-hook--pre-commit.sh
> @@ -0,0 +1,75 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Christian Höltje
> +#
> +
> +test_description='t7503 templates-hooks--pre-commit
> +
> +This test verifies that the pre-commit hook shipped with
> +git by default works correctly.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'verify that autocrlf is unset' '
> + if git config core.autocrlf
> + then
> + false
> + else
> + test $? -eq 1
> + fi
> +'
> +
> +test_expect_success 'lf without hook' '
> +
> + echo "foo" > lf.txt &&
> + git add lf.txt &&
> + git commit -m "lf without hook" lf.txt
> +
> +'
> +
> +test_expect_success 'crlf without hook' '
> +
> + echo "foo\r" > crlf.txt &&
Perhaps you want to use printf "foo\r" instead? echo "foo\r" does not
produce a CR on my system.
> + git add crlf.txt &&
> + git commit -m "crlf without hook" crlf.txt
> +
> +'
> +
> +# Set up the pre-commit hook.
> +HOOKDIR="$(git rev-parse --git-dir)/hooks"
> +mkdir -p "${HOOKDIR}"
> +cp -r "${HOOKDIR}-disabled/pre-commit" "${HOOKDIR}/pre-commit"
> +chmod +x "${HOOKDIR}/pre-commit"
> +
> +test_expect_success 'lf with hook' '
> +
> + echo "bar" >> lf.txt &&
> + git add lf.txt &&
> + git commit -m "lf with hook" lf.txt
> +
> +'
> +test_expect_success 'crlf with hook' '
> +
> + echo "bar\r" >> crlf.txt &&
> + git add crlf.txt &&
> + git commit -m "crlf with hook" crlf.txt
> +
> +'
> +
> +test_expect_success 'lf with hook white-space failure' '
> +
> + echo "bar " >> lf.txt &&
> + git add lf.txt &&
> + ! git commit -m "lf with hook" lf.txt
> +
> +'
> +test_expect_success 'crlf with hook white-space failure' '
> +
> + echo "bar \r" >> crlf.txt &&
> + git add crlf.txt &&
> + ! git commit -m "crlf with hook" crlf.txt
> +
> +'
> +
> +test_done
> diff --git a/templates/hooks--pre-commit b/templates/hooks--pre-commit
> index b25dce6..335ca09 100644
> --- a/templates/hooks--pre-commit
> +++ b/templates/hooks--pre-commit
> @@ -55,8 +55,14 @@ perl -e '
> if (s/^\+//) {
> $lineno++;
> chomp;
> - if (/\s$/) {
> - bad_line("trailing whitespace", $_);
> + if (/\r$/) {
Wouldn't it be less redundant to do a test for \s\r$ here instead of
testing for \r$ and then \s\r$?
> + if (/\s\r$/) {
> + bad_line("trailing whitespace", $_);
> + }
> + } else {
> + if (/\s$/) {
> + bad_line("trailing whitespace", $_);
> + }
> }
> if (/^\s* \t/) {
> bad_line("indent SP followed by a TAB", $_);
>
--
Ian Hilt
Ian.Hilt (at) gmx.com
GnuPG key: 0x4AFC1EE3
^ permalink raw reply
* Re: [PATCH] pre-commit hook should ignore carriage returns at EOL
From: Alf Clement @ 2008-06-24 18:22 UTC (permalink / raw)
To: Christian Holtje; +Cc: git, gitster
In-Reply-To: <53A5AFCF-94C7-465E-A181-1DA69F251F5B@gmail.com>
Hi Christian,
thanks for the patch. I use git under Windows and also run often into
these problems,
because I have to (but don't like to) use come compilers under Windows.
I usually comment the two bad_lines() in the pre-commit-hook out by hand:
"trailing whitespace" and "indent SP followed by TAB", because i.e.
Visual Studio writes some files out, which trigger these checks.
Can't we get rid of these checks?
CU,
Alf
On 6/24/08, Christian Holtje <docwhat@gmail.com> wrote:
> When commit files that use DOS style CRLF end-of-lines, the pre-commit
> hook would raise an error. When combined with the fact that the hooks
> get activated by default on windows, it makes life difficult for
> people working with DOS files.
>
> This patch causes the pre-commit hook to deal with crlf files
> correctly.
>
> Signed-off-by: Christian Höltje <docwhat@gmail.com>
> ---
> t/t7503-template-hook--pre-commit.sh | 75 +++++++++++++++++++++++++
> +++++++++
> templates/hooks--pre-commit | 10 ++++-
> 2 files changed, 83 insertions(+), 2 deletions(-)
> create mode 100755 t/t7503-template-hook--pre-commit.sh
>
> diff --git a/t/t7503-template-hook--pre-commit.sh b/t/t7503-template-
> hook--pre-commit.sh
> new file mode 100755
> index 0000000..8f0c3c9
> --- /dev/null
> +++ b/t/t7503-template-hook--pre-commit.sh
> @@ -0,0 +1,75 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Christian Höltje
> +#
> +
> +test_description='t7503 templates-hooks--pre-commit
> +
> +This test verifies that the pre-commit hook shipped with
> +git by default works correctly.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'verify that autocrlf is unset' '
> + if git config core.autocrlf
> + then
> + false
> + else
> + test $? -eq 1
> + fi
> +'
> +
> +test_expect_success 'lf without hook' '
> +
> + echo "foo" > lf.txt &&
> + git add lf.txt &&
> + git commit -m "lf without hook" lf.txt
> +
> +'
> +
> +test_expect_success 'crlf without hook' '
> +
> + echo "foo\r" > crlf.txt &&
> + git add crlf.txt &&
> + git commit -m "crlf without hook" crlf.txt
> +
> +'
> +
> +# Set up the pre-commit hook.
> +HOOKDIR="$(git rev-parse --git-dir)/hooks"
> +mkdir -p "${HOOKDIR}"
> +cp -r "${HOOKDIR}-disabled/pre-commit" "${HOOKDIR}/pre-commit"
> +chmod +x "${HOOKDIR}/pre-commit"
> +
> +test_expect_success 'lf with hook' '
> +
> + echo "bar" >> lf.txt &&
> + git add lf.txt &&
> + git commit -m "lf with hook" lf.txt
> +
> +'
> +test_expect_success 'crlf with hook' '
> +
> + echo "bar\r" >> crlf.txt &&
> + git add crlf.txt &&
> + git commit -m "crlf with hook" crlf.txt
> +
> +'
> +
> +test_expect_success 'lf with hook white-space failure' '
> +
> + echo "bar " >> lf.txt &&
> + git add lf.txt &&
> + ! git commit -m "lf with hook" lf.txt
> +
> +'
> +test_expect_success 'crlf with hook white-space failure' '
> +
> + echo "bar \r" >> crlf.txt &&
> + git add crlf.txt &&
> + ! git commit -m "crlf with hook" crlf.txt
> +
> +'
> +
> +test_done
> diff --git a/templates/hooks--pre-commit b/templates/hooks--pre-commit
> index b25dce6..335ca09 100644
> --- a/templates/hooks--pre-commit
> +++ b/templates/hooks--pre-commit
> @@ -55,8 +55,14 @@ perl -e '
> if (s/^\+//) {
> $lineno++;
> chomp;
> - if (/\s$/) {
> - bad_line("trailing whitespace", $_);
> + if (/\r$/) {
> + if (/\s\r$/) {
> + bad_line("trailing whitespace", $_);
> + }
> + } else {
> + if (/\s$/) {
> + bad_line("trailing whitespace", $_);
> + }
> }
> if (/^\s* \t/) {
> bad_line("indent SP followed by a TAB", $_);
> --
> 1.5.5.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: Brandon Casey @ 2008-06-24 18:18 UTC (permalink / raw)
To: Boaz Harrosh; +Cc: David Jeske, git
In-Reply-To: <48612ABE.6000104@panasas.com>
Boaz Harrosh wrote:
> I use git reset --hard in to separate and distinct functions.
> One - to move current branch head around from place to place.
Why?
> Two - Throw away work I've edited
This is valid.
> It has happened to me more then once that I wanted the first
> and also got the second as an un-warned bonus, to the dismay
> of my bosses.
Why are you using 'git reset' to do this? Why not just checkout
the branch? I think you are using 'reset' in ways it is not
intended to be used. Is there something in the documentation that
led you to believe that 'reset --hard' should be used to switch
branches? I do see an example of such a thing in everyday.txt.
It deals with setting 'pu' branch to the tip of the 'next' branch,
but the 'pu' branch has a special meaning in git.
It seems like you are using 'reset' when you should be using 'checkout'.
For example:
$ git branch
* mybranch
master
next
maint
pu
If I have 'mybranch' checked out and I want to make a change on top of
the 'next' branch, I wouldn't do 'git reset --hard next', I would either
'git checkout next' or 'git checkout -b next-feature next' or something
similar.
If I've already merged the changes from mybranch back into upstream, then
it's safe to delete it.
I recommend adopting a branch naming scheme where the branch name describes
the task that is to be accomplished. i.e. 'foo' is a bad branch name.
btw, you are not saving anything by trying to reuse branch names. All
a branch is, is a file with a 40 byte string and a newline. So creating
a branch entails writing 41 bytes to a file. Deleting a branch entails
deleting a single file that is only 41 bytes small.
I suggest trying to adjust your work flow so that 'reset --hard' is not necessary.
-brandon
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Linus Torvalds @ 2008-06-24 17:44 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Pierre Habouzit, Git Mailing List,
Junio C Hamano
In-Reply-To: <20080624173428.GA9500@sigill.intra.peff.net>
On Tue, 24 Jun 2008, Jeff King wrote:
>
> Perhaps I was confused about the definition of "single", because
> throughout this thread you seem to be making multiple complaints about
> parse_options, including its lack of a "stop on unknown" flag, a
> "continue on unknown flag", and the movement of arguments within the
> argv array.
You think that is multiple problems, but it's not.
It was a single issue - the issue of being able to do incremental parsing,
and mixing *different* parsers together (not just two different calls to
"parse_options()", but also having hand-parsing still in the picture for
things where it was hard to convert).
And to solve that _single_ problem, I wanted parse_options() to be able
to:
- stop at unknown options (so that I could hand-parse them)
- ignore unknown options (so that I could parse all the ones I knew
about, and then either hand-parse the rest, or just pass them on to
_another_ function that used some arbitrary model to parse the parts it
knew about)
See? Single issue.
And I even sent out a single patch for it. That single patch, btw, was
even rather small.
Did you ever look at that patch? Did you ever look at the code I was
trying to have use parse_options()? No.
> So I will say one last time, as clearly as I possibly can, what I was
> trying to bring to the discussion:
You constantly try to change the discussion to be about SOMETHIGN ELSE.
For example, you keep on bringing up this TOTAL RED HERRING:
> - It is impossible for that mechanism to be correct in all cases, due
> to the syntactic rules for command lines. IOW, you cannot parse an
> element until you know the function of the element to the left.
NOBODY F*CKIGN CARES!
Because what builtin-blame.c *already* does is exactly that.
This is what I'm complaining about with your totally IDIOTIC mails. You're
ignoring reality, and talking about how things "ought to work", and never
ever apparently looked at how things *do* work.
The fact is, the one program I wanted to convert already does exactly what
you claim is "impossible to be correct in all cases".
So either shut up, or send a patch to fix what you consider a bug. I'm
waiting.
So screw it. I'm simply not interested in discussing this with you. You
aren't apparently interested in looking at the problems we have today,
because you're only interested in fixing some theoretical case.
Linus
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 16:41 UTC (permalink / raw)
To: Brandon Casey; +Cc: git
In-Reply-To: <U-ySqQANiPRpld4kgzdXbovGgsj6LfOEdRmtTDU2yyvITSG3LnZAsQ@cipher.nrlssc.navy.mil>
My takeaways from this thread:
- THANKS! to all of you for the detailed discussion, and for making git. Even
though it's still unfamiliar to me, I really enjoy (g)it!
- I don't think anyone here thinks git is beyond improvement. This discussion
did change my mind on a few things since my original post. I started this
discussion to share my "unacclimated usability suggestions", because after I
acclimate to git, I'll be telling new users that these idiosyncrasies are all
no big deal too. :) I still think there is value in this list of suggestions.
I'll work on submitting patches...
- improve the man page description of "reset --hard" (see below)
- standardize all the potentially destructive operations (after gc) on "-f /
--force" to override
- add "checkout" to the git-gui history right-click menu, and make the danger
of
"reset --hard" more obvious and require a confirmation dialog (the gui
equivilant of -f)
----------
a couple more specific responses below..
-- Rogan Dawes wrote:
> -- David wrote:
> > Let me guess, you're always running euid==0. :)
> Do you also ask the gnu coreutils folks to remove the -f option from their
utilities?
-- Johannes Gilger wrote:
> I think the name of the command "reset" itself is a name which should
> prompt everyone to read a manpage before using it. [snip ]
> Nobody complains about rm --force or anything.
Isn't it nice that they standardized on "-f" and "--force" across ALL commands?
I would be inclined to talk to coreutils if it was "rm -f", "cp -R" (vs cp -r),
and "mv --aggressive" to do the respective non-safe versions.
It would simplify git's command-line-ui and cognitive load if it did the same
thing. Pick one standard for "overriding dangerous commands", instead of
"danger caps" and "danger --reset" and "danger -f". Consider branch which has
both "branch -[MD]" and "branch -f" in the same subcommand. What's wrong with
"branch -[md] -f"?
Of course --hard encourages one to read the manpage. However, git is using a
bunch of new terms for things, and uses at least those three different methods
to indicate command danger. Lets look at the working on the manpage:
"Matches the working tree and index to that of the tree being
switched
to. Any changes to tracked files in the working tree since <commit>
are lost."
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I interpreted this as "any [non committed changes] to tracked files in the
working tree since <commit> are lost." I don't this this was a naive
interpretation. I still think that's the way it reads after this whole
conversation.
I'll work on my first patch for git:
-> "References to any working tree changes, and pulled changes, AND COMMITTED
CHANGES to tracked files in the branch after <commit> will be dropped, causing
them to be removed at the next garbage collect.".
-- Brandon Casey wrote:
> After saying all of that, here is how I think you _should_ have done things.
> Notice I _did_not_ use 'reset --hard'.
I was told that I can safely do "git checkout origin/master" instead of "reset
--hard" to get back to the pull point, in case I didn't branch ahead of time.
The wrinkle being that my "master" branch-pointer still points to my local
changes, so I need to move onto a different branchname before I push if I want
to avoid those changes going to the server, which is fine..
> git clone <master_repo>
> cd master_repo
> git checkout -b feature1 # we create our feature branch immediately since
> # creating branches is so effortless in git. A
> # private feature branch should _always_ be created
> # and used for development.
I'm beginning to see why I would always work this way, though if "private
feature branches should always be created and used for development", then I'm
unclear about why this isn't the default. git could implicitly create them when
I checkin a change on the head of a pulled branch. (i.e. user/branchname/id, or
something else). I'm reaching here, I'll need to use git more with other
developers to understand this better.
-------------------
Thanks again for all the detailed responses and explanations!
- David
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 16:41 UTC (permalink / raw)
To: Brandon Casey; +Cc: git
In-Reply-To: <U-ySqQANiPRpld4kgzdXbovGgsj6LfOEdRmtTDU2yyvITSG3LnZAsQ@cipher.nrlssc.navy.mil>
My takeaways from this thread:
- THANKS! to all of you for the detailed discussion, and for making git. Even
though it's still unfamiliar to me, I really enjoy (g)it!
- I don't think anyone here thinks git is beyond improvement. This discussion
did change my mind on a few things since my original post. I started this
discussion to share my "unacclimated usability suggestions", because after I
acclimate to git, I'll be telling new users that these idiosyncrasies are all
no big deal too. :) I still think there is value in this list of suggestions.
I'll work on submitting patches...
- improve the man page description of "reset --hard" (see below)
- standardize all the potentially destructive operations (after gc) on "-f /
--force" to override
- add "checkout" to the git-gui history right-click menu, and make the danger
of
"reset --hard" more obvious and require a confirmation dialog (the gui
equivilant of -f)
----------
a couple more specific responses below..
-- Rogan Dawes wrote:
> -- David wrote:
> > Let me guess, you're always running euid==0. :)
> Do you also ask the gnu coreutils folks to remove the -f option from their
utilities?
-- Johannes Gilger wrote:
> I think the name of the command "reset" itself is a name which should
> prompt everyone to read a manpage before using it. [snip ]
> Nobody complains about rm --force or anything.
Isn't it nice that they standardized on "-f" and "--force" across ALL commands?
I would be inclined to talk to coreutils if it was "rm -f", "cp -R" (vs cp -r),
and "mv --aggressive" to do the respective non-safe versions.
It would simplify git's command-line-ui and cognitive load if it did the same
thing. Pick one standard for "overriding dangerous commands", instead of
"danger caps" and "danger --reset" and "danger -f". Consider branch which has
both "branch -[MD]" and "branch -f" in the same subcommand. What's wrong with
"branch -[md] -f"?
Of course --hard encourages one to read the manpage. However, git is using a
bunch of new terms for things, and uses at least those three different methods
to indicate command danger. Lets look at the working on the manpage:
"Matches the working tree and index to that of the tree being
switched
to. Any changes to tracked files in the working tree since <commit>
are lost."
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I interpreted this as "any [non committed changes] to tracked files in the
working tree since <commit> are lost." I don't this this was a naive
interpretation. I still think that's the way it reads after this whole
conversation.
I'll work on my first patch for git:
-> "References to any working tree changes, and pulled changes, AND COMMITTED
CHANGES to tracked files in the branch after <commit> will be dropped, causing
them to be removed at the next garbage collect.".
-- Brandon Casey wrote:
> After saying all of that, here is how I think you _should_ have done things.
> Notice I _did_not_ use 'reset --hard'.
I was told that I can safely do "git checkout origin/master" instead of "reset
--hard" to get back to the pull point, in case I didn't branch ahead of time.
The wrinkle being that my "master" branch-pointer still points to my local
changes, so I need to move onto a different branchname before I push if I want
to avoid those changes going to the server, which is fine..
> git clone <master_repo>
> cd master_repo
> git checkout -b feature1 # we create our feature branch immediately since
> # creating branches is so effortless in git. A
> # private feature branch should _always_ be created
> # and used for development.
I'm beginning to see why I would always work this way, though if "private
feature branches should always be created and used for development", then I'm
unclear about why this isn't the default. git could implicitly create them when
I checkin a change on the head of a pulled branch. (i.e. user/branchname/id, or
something else). I'm reaching here, I'll need to use git more with other
developers to understand this better.
-------------------
Thanks again for all the detailed responses and explanations!
- David
^ permalink raw reply
* Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'
From: Jeff King @ 2008-06-24 17:41 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0806241808400.9925@racer>
On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:
> And this is my first attempt at a proper patch for it.
>
> Now with documentation, and hopefully all places where the
> user is being told about a "bad" commit.
This looks reasonably sane to me. The only thing I can think of that
we're missing is that "git bisect visualize" will still show the refs as
"bisect/bad" and "bisect/good".
To fix that, you'd have to ask people to start the bisect by saying "I
am bisecting to find a fix, not a breakage." And then you could change
the refnames and all of the messages as appropriate.
-Peff
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Jeff King @ 2008-06-24 17:34 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Pierre Habouzit, Git Mailing List,
Junio C Hamano
In-Reply-To: <alpine.LFD.1.10.0806240954150.2926@woody.linux-foundation.org>
On Tue, Jun 24, 2008 at 09:59:47AM -0700, Linus Torvalds wrote:
> I have a _single_ problem I have with parse_options(), namely that it was
> painful to convert in pieces. It may well be that builtin-blame.c was one
> of the more painful cases, but it really was a _single_ issue.
>
> I also had a _single_ fix for it.
>
> I never had "other" problems.
>
> What happened was that you and Dscho and others then tried to pick that
> _single_ issue apart, because the solutions _you_ wanted (tying all the
Perhaps I was confused about the definition of "single", because
throughout this thread you seem to be making multiple complaints about
parse_options, including its lack of a "stop on unknown" flag, a
"continue on unknown flag", and the movement of arguments within the
argv array.
But whether you want to call that a "single" problem or not, my point
was that I am not talking about most of those things.
So I will say one last time, as clearly as I possibly can, what I was
trying to bring to the discussion:
- You proposed a CONTINUE_ON_UNKNOWN type of flag.
- It is impossible for that mechanism to be correct in all cases, due
to the syntactic rules for command lines. IOW, you cannot parse an
element until you know the function of the element to the left.
- I wanted to mention it specifically, because that exact mechanism
had already been proposed in a patch last week, and Junio said "this
conceptually is broken".
- There has been discussion underway about what is the best mechanism
to solve the same situation.
That is the entirety of my point. I am glad you are trying to increase
parse_options uptake. There is obviously a problem with multi-stage
parsers. I talked about one way for them to be handled. I think there
are multiple ways of going about it. It looks like STOP_ON_UNKNOWN
is the way that Pierre is pursuing. I think this is good, because it
doesn't suffer from the corner cases that CONTINUE_ON_UNKNOWN does.
And now I will stop making these points, because I don't think I am
capable of saying them any more clearly than I already have, and because
Pierre seems to be moving in a sane direction.
> And then you talk about how things "ought to be" in your world, to make
> your solution relevant at all.
>
> And I'm trying to tell you that "ought to be" has no relevance, because
> you're not even looking at the problem!
Again, did you even read the mail you are responding to? The phrase
"ought to be" was totally incidental to the point I was making. I could
just as easily have said "and this is the method that I think will be
acceptable for dealing with this problem." But for some reason you
insist on harping on the phrase as if I have proposed magical fairies
should come work on the code, and totally ignoring the actual points
that were made.
-Peff
^ permalink raw reply
* Re: [PATCH 5/7] parse-opt: fake short strings for callers to believe in.
From: Linus Torvalds @ 2008-06-24 17:20 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, gitster, peff, Johannes.Schindelin
In-Reply-To: <1214298732-6247-6-git-send-email-madcoder@debian.org>
On Tue, 24 Jun 2008, Pierre Habouzit wrote:
>
> If we begin to parse -abc and that the parser knew about -a and -b, it
> will fake a -c switch for the caller to deal with.
>
> Of course in the case of -acb (supposing -c is not taking an argument) the
> caller will have to be especially clever to do the same thing. We could
> think about exposing an API to do so if it's really needed, but oh well...
Well, if the other parser is _also_ parse_options() (ie you just cascade
them incrementally in a loop), then the other parser should get it right
automatically. No?
Linus
^ permalink raw reply
* Re: [PATCH 6/7] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.
From: Linus Torvalds @ 2008-06-24 17:18 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, gitster, peff, Johannes.Schindelin
In-Reply-To: <1214298732-6247-7-git-send-email-madcoder@debian.org>
On Tue, 24 Jun 2008, Pierre Habouzit wrote:
>
> This way, argv[0] isn't clobbered, to the cost of maybe not having a
> resulting NULL terminated argv array.
Umm. I think it's much easier to do by always having
ctx->out = argv;
and then just initializing cpix to 0 or 1:
ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
because now parse_options_end() doesn't need to play games any more. It
doesn't need to care about PARSE_OPT_KEEP_ARGV0, it can just do exactly
what it always used to do, because "ctx->cpidx + ctx->argc" automatically
does the right thing (it is both the return value _and_ the index that you
should fill with NULL.
Hmm?
Linus
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: Boaz Harrosh @ 2008-06-24 17:19 UTC (permalink / raw)
To: David Jeske; +Cc: git
In-Reply-To: <48612ABE.6000104@panasas.com>
Boaz Harrosh wrote:
> David Jeske wrote:
>> As a new user, I'm finding git difficult to trust, because there are operations
>> which are destructive by default and capable of inadvertently throwing hours or
>> days of work into the bit bucket.
>>
>> More problematic, those commands have no discernible pattern that shows their
>> danger, and they need to be used to do typical everyday things. I'm starting to
>> feel like I need to use another source control system on top of the git
>> repository in case I make a mistake. My philosophy is simple, I never never
>> never want to throw away changes, you shouldn't either. Disks are cheaper than
>> programmer hours. I can understand wanting to keep things tidy, so I can
>> understand ways to correct the 'easily visible changes', and also avoid pushing
>> them to other trees, but I don't understand why git needs to delete things.
>>
>> For example, the following commands seem capable of totally destroying hours or
>> days of work. Some of them need to be used regularly to do everyday things, and
>> there is no pattern among them spelling out danger.
>>
>> git reset --hard : if another branch name hasn't been created
>
> git reset --hard is special see below
>
>> git rebase
>> git branch -D <branch> : if branch hasn't been merged
>> git branch -f <new> : if new exists and hasn't been merged
>> git branch -m <old> <new> : if new exists and hasn't been merged
>>
> The rest of the commands are recoverable from the log as people said
> but "git reset --hard" is not and should be *fixed*!
>
> I use git reset --hard in to separate and distinct functions.
> One - to move current branch head around from place to place.
> Two - Throw away work I've edited
>
> It has happened to me more then once that I wanted the first
> and also got the second as an un-warned bonus, to the dismay
> of my bosses. (What do I care if I need to write all this code
> again)
>
> I would like git-reset --hard to refuse if a git-diff HEAD
> (both staged and unstaged) is not empty. with a -f / -n logic
> like git-clean. (like git-clean none default config file override)
>
> Now I know that the first usage above could be done with
> git-branch -f that_branch the_other_branch. But that can
> not be preformed on the current branch and local changes
> are not lost.
>
> Lots of other potentially destructive git-commands check for local
> changes and refuse to operate. To remedy them git-reset --hard
> is recommended. I would prefer if there was a git-reset --clean -f/-n
> for the first case and git reset --hard only for the second usage
> case.
Sorry
git-reset --clean -f/-n for removing local changes
git reset --hard for moving HEAD on a clean tree only
>
> My $0.017
> Boaz
>
^ 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