From: John Keeping <john@keeping.me.uk>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: [PATCH] add--interactive: respect diff.algorithm
Date: Wed, 12 Jun 2013 19:44:10 +0100 [thread overview]
Message-ID: <20130612184410.GB23890@serenity.lan> (raw)
In-Reply-To: <20130610215656.GA28345@sigill.intra.peff.net>
When staging hunks interactively it is sometimes useful to use an
alternative diff algorithm which splits the changes into hunks in a more
logical manner. This is not possible because the plumbing commands
called by add--interactive ignore the "diff.algorithm" configuration
option (as they should).
Since add--interactive is a porcelain command it should respect this
configuration variable. To do this, make it read diff.algorithm and
pass its value to the underlying diff-index and diff-files invocations.
At this point, do not add options to "git add", "git reset" or "git
checkout" (all of which can call git-add--interactive). If a user want
to override the value on the command line they can use:
git -c diff.algorithm=$ALGO ...
Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Mon, Jun 10, 2013 at 05:56:56PM -0400, Jeff King wrote:
> On Mon, Jun 10, 2013 at 10:46:38PM +0100, John Keeping wrote:
>
> > > Overall, I think respecting diff.algorithm in add--interactive is a very
> > > sane thing to do. I would even be tempted to say we should allow a few
> > > other select diff options (e.g., fewer or more context lines). If you
> > > allowed diff options like this:
> > >
> > > git add --patch="--patience -U5"
> > >
> > > that is very flexible, but I would not want to think about what the code
> > > does when you pass --patch="--raw" or equal nonsense.
> >
> > An alternative would be to permit them to be set from within the
> > interactive UI. I'd find it quite useful to experiment with various
> > diff options when I encounter a hunk that isn't as easy to pick as I'd
> > like. I expect it would be very hard to do that on a per-hunk basis,
> > although per-file doesn't seem like it would be too hard.
>
> That's an interesting idea, for a subset of options (e.g., "increase
> context for this hunk"). I suspect implementing it would be painful,
> though, as you would have to re-run diff, and you have no guarantee of
> getting the same set of hunks (e.g., the hunk might end up coalesced
> with another).
I think you'd need to re-run the diff over the whole file and then skip
hunks until you reach one that overlaps with the original hunk. But I
suspect it would end up being quite a lot more complicated than that.
> > diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> > index d2c4ce6..0b0fac2 100755
> > --- a/git-add--interactive.perl
> > +++ b/git-add--interactive.perl
> > @@ -44,6 +44,8 @@ my ($diff_new_color) =
> >
> > my $normal_color = $repo->get_color("", "reset");
> >
> > +my $diff_algorithm = ($repo->config('diff.algorithm') or 'default');
> > +
> > my $use_readkey = 0;
> > my $use_termcap = 0;
> > my %term_escapes;
> > @@ -731,6 +733,9 @@ sub run_git_apply {
> > sub parse_diff {
> > my ($path) = @_;
> > my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
> > + if ($diff_algorithm ne "default") {
> > + push @diff_cmd, "--diff-algorithm=${diff_algorithm}";
> > + }
> > if (defined $patch_mode_revision) {
> > push @diff_cmd, $patch_mode_revision;
>
> Yeah, that looks like the sane way to do it to me. As a perl style
> thing, I think the usual way of spelling 'default' is 'undef'. I.e.:
>
> my $diff_algorithm = $repo->config('diff.algorithm');
> ...
> if (defined $diff_algorithm) {
> push @diff_cmd, "--diff-algorithm=$diff_algorithm";
> }
OK. The "default" is actually "the value that is equivalent to 'myers'
for diff.algorithm" and I was originally going to add --diff-algorithm
to the command line unconditionally.
But I think it's better to add it only when it has been specified so
I've changed it as you suggest.
git-add--interactive.perl | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d2c4ce6..5310959 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -44,6 +44,8 @@ my ($diff_new_color) =
my $normal_color = $repo->get_color("", "reset");
+my $diff_algorithm = $repo->config('diff.algorithm');
+
my $use_readkey = 0;
my $use_termcap = 0;
my %term_escapes;
@@ -731,6 +733,9 @@ sub run_git_apply {
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+ if (defined $diff_algorithm) {
+ push @diff_cmd, "--diff-algorithm=${diff_algorithm}";
+ }
if (defined $patch_mode_revision) {
push @diff_cmd, $patch_mode_revision;
}
--
1.8.3.779.g691e267
next prev parent reply other threads:[~2013-06-12 18:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-10 14:28 Different diff strategies in add --interactive John Keeping
2013-06-10 19:28 ` Junio C Hamano
2013-06-10 21:11 ` Jeff King
2013-06-10 21:46 ` John Keeping
2013-06-10 21:56 ` Jeff King
2013-06-12 18:44 ` John Keeping [this message]
2013-06-12 19:18 ` [PATCH] add--interactive: respect diff.algorithm Jeff King
2013-06-23 19:19 ` Junio C Hamano
2013-06-23 19:50 ` John Keeping
2013-06-23 20:37 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130612184410.GB23890@serenity.lan \
--to=john@keeping.me.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).