On Tue, Jul 26, 2011 at 10:03:31AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Tue, Jul 26, 2011 at 4:55 AM, Jeff King <peff@xxxxxxxx> wrote:
> > On Sun, Jul 24, 2011 at 12:11:29PM +0700, Nguyen Thai Ngoc Duy wrote:
> >
> >> Can we have "git add--interactive --patch --match=regex" where only
> >> (splitted if possible) hunks that match regex are shown?
> >
> > The patch below does this,
>
> Thanks!
So I did this as a quick "that should be really easy" proof of concept.
I'm not too interested personally in taking it all the way to an
acceptable patch. I'm happy to help out with the perl parts, though, if
you want to do the rest (C scaffolding for calling add--interactive,
tests, and docs).
> > 1. What does it mean to be "shown"? My patch auto-marks non-matching
> > hunks as "do not stage". That means you can still switch back to
> > them in the hunk list and enable them if you want. Another option
> > would be to omit them entirely, and pretend that those hunks don't
> > exist.
>
> Either way is ok. Maybe the option in this case could be changed to
> --nostage=regex to reflect the behavior.
I can't help but think there is some way to relate it to the
path-limiting that we do elsewhere. It's sort of like diffcore's "-G",
except that is about picking whole files, not individual hunks. I guess
most of git doesn't operate at the hunk level in quite the same way, so
there is no analagous part.
The more I think about it, it is probably simpler both conceptually and
in implementation to filter out those hunks entirely instead of marking
them used. The latter gives the user a chance to manually find them and
go back to them via 'J' and 'K'. But I find the chance that that is
useful to be much less than the chance that somebody gets confused or
annoyed by them being there (because they were iterating, or because
they did a "/" search).
And it's not like everything needs to be in a single "add" call. The
point of "add" is that you could call it multiple times, or even call
it, then check what "git diff" says, then decide you want to stage some
more.
> > 2. What should we do with non-text changes, like mode changes are
> > full-file deletion?
>
> Probably invalid use case for --match.
Invalid, like we should have an error? I think that would be annoying,
because you want to be able to do "git add -p --match=foo" even when
there is an unrelated mode change. I think in the mindset of "this is a
filter for things you want to see", it probably makes sense to just
pretend they aren't there (and the user can always follow up with
another "add", as I mentioned above).
> > 3. What should be shown for a file with no matching hunks? Probably
> > nothing (i.e., as if it had been limited away by pathname)? My
> > patch shows the header, but that is easy to fix.
>
> Printing "Nothing to add" would be nice.
If we treat it like path limiting, I think we could just print nothing
for that file. That is, doing:
git add -p foo
would not even look at "bar" or print anything about it. Similarly:
git add -p --match=foo
should probably say nothing whatsoever about files that didn't mention
foo at all.
For reference, here's the C scaffolding I used to test the patch I sent
earlier. I didn't include it earlier because it made the perl bits hard
to find amidst all of the changes.
Probably the other interactive modes (checkout, reset, etc) should get a
similar option; it would just be a matter of passing the regex through
to the perl script.
---
diff --git a/builtin/add.c b/builtin/add.c
index 822ee3a..096dbc7 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -220,20 +220,25 @@ static const char **validate_pathspec(int argc, const char **argv, const char *p
}
int run_add_interactive(const char *revision, const char *patch_mode,
- const char **pathspec)
+ const char *match, const char **pathspec)
{
int status, ac, pc = 0;
const char **args;
+ struct strbuf match_arg = STRBUF_INIT;
if (pathspec)
while (pathspec[pc])
pc++;
- args = xcalloc(sizeof(const char *), (pc + 5));
+ args = xcalloc(sizeof(const char *), (pc + 6));
ac = 0;
args[ac++] = "add--interactive";
if (patch_mode)
args[ac++] = patch_mode;
+ if (match) {
+ strbuf_addf(&match_arg, "--match=%s", match);
+ args[ac++] = match_arg.buf;
+ }
if (revision)
args[ac++] = revision;
args[ac++] = "--";
@@ -245,10 +250,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,
status = run_command_v_opt(args, RUN_GIT_CMD);
free(args);
+ strbuf_release(&match_arg);
return status;
}
-int interactive_add(int argc, const char **argv, const char *prefix, int patch)
+int interactive_add(int argc, const char **argv, const char *prefix, int patch,
+ const char *match)
{
const char **pathspec = NULL;
@@ -260,7 +267,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch)
return run_add_interactive(NULL,
patch ? "--patch" : NULL,
- pathspec);
+ match, pathspec);
}
static int edit_patch(int argc, const char **argv, const char *prefix)
@@ -317,6 +324,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;
+static const char *patch_match;
static struct option builtin_add_options[] = {
OPT__DRY_RUN(&show_only, "dry run"),
@@ -324,6 +332,8 @@ static struct option builtin_add_options[] = {
OPT_GROUP(""),
OPT_BOOLEAN('i', "interactive", &add_interactive, "interactive picking"),
OPT_BOOLEAN('p', "patch", &patch_interactive, "select hunks interactively"),
+ OPT_STRING(0, "match", &patch_match, "regex",
+ "find pattern within --patch hunks"),
OPT_BOOLEAN('e', "edit", &edit_interactive, "edit current diff and apply"),
OPT__FORCE(&ignored_too, "allow adding otherwise ignored files"),
OPT_BOOLEAN('u', "update", &take_worktree_changes, "update tracked files"),
@@ -384,7 +394,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (patch_interactive)
add_interactive = 1;
if (add_interactive)
- exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));
+ exit(interactive_add(argc - 1, argv + 1, prefix,
+ patch_interactive, patch_match));
if (edit_interactive)
return(edit_patch(argc, argv, prefix));
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d647a31..8319bb2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -765,7 +765,8 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
static int interactive_checkout(const char *revision, const char **pathspec,
struct checkout_opts *opts)
{
- return run_add_interactive(revision, "--patch=checkout", pathspec);
+ return run_add_interactive(revision, "--patch=checkout", NULL,
+ pathspec);
}
struct tracking_name_data {
diff --git a/builtin/commit.c b/builtin/commit.c
index e50694e..24d90a4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -366,7 +366,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
old_index_env = getenv(INDEX_ENVIRONMENT);
setenv(INDEX_ENVIRONMENT, index_lock.filename, 1);
- if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
+ if (interactive_add(argc, argv, prefix, patch_interactive,
+ NULL) != 0)
die(_("interactive add failed"));
if (old_index_env && *old_index_env)
diff --git a/builtin/reset.c b/builtin/reset.c
index 811e8e2..924c104 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -165,7 +165,7 @@ static int interactive_reset(const char *revision, const char **argv,
if (*argv)
pathspec = get_pathspec(prefix, argv);
- return run_add_interactive(revision, "--patch=reset", pathspec);
+ return run_add_interactive(revision, "--patch=reset", NULL, pathspec);
}
static int read_from_tree(const char *prefix, const char **argv,
diff --git a/commit.h b/commit.h
index b23f33b..5ab0072 100644
--- a/commit.h
+++ b/commit.h
@@ -167,9 +167,10 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit **, int);
-extern int interactive_add(int argc, const char **argv, const char *prefix, int patch);
+extern int interactive_add(int argc, const char **argv, const char *prefix,
+ int patch, const char *match);
extern int run_add_interactive(const char *revision, const char *patch_mode,
- const char **pathspec);
+ const char *match, const char **pathspec);
static inline int single_parent(struct commit *commit)
{
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 2ee0908..07896d4 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -86,6 +86,7 @@ sub colored {
# command line options
my $patch_mode;
my $patch_mode_revision;
+my $patch_match;
sub apply_patch;
sub apply_patch_for_checkout_commit;
@@ -1277,6 +1278,17 @@ sub display_hunks {
return $i;
}
+sub want_hunk {
+ my ($re, $hunk) = @_;
+
+ return 1 if $hunk->{TYPE} ne 'hunk';
+
+ foreach my $line (@{$hunk->{TEXT}}) {
+ return 1 if $line =~ $re;
+ }
+ return 0;
+}
+
sub patch_update_file {
my $quit = 0;
my ($ix, $num);
@@ -1301,6 +1313,20 @@ sub patch_update_file {
$num = scalar @hunk;
$ix = 0;
+ if ($patch_match) {
+ # mark non-matching text hunks as "do not want"
+ foreach my $hunk (@hunk) {
+ if (!want_hunk($patch_match, $hunk)) {
+ $hunk->{USE} = 0;
+ }
+ }
+ # and then advance us to the first undecided hunk
+ while ($ix < $num) {
+ last unless defined $hunk[$ix]{USE};
+ $ix++;
+ }
+ }
+
while (1) {
my ($prev, $next, $other, $undecided, $i);
$other = '';
@@ -1606,6 +1632,10 @@ sub process_args {
} else {
$patch_mode = 'stage';
$arg = shift @ARGV or die "missing --";
+ if ($arg =~ /--match=(.*)/) {
+ $patch_match = qr/$1/;
+ $arg = shift @ARGV or die "missing --";
+ }
}
die "invalid argument $arg, expecting --"
unless $arg eq "--";
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html