Re: [PATCH] add--interactive: respect diff.compactionHeuristic

From: Stefan Beller <sbeller@google.com>
Date: Jun 16, 2016, 16:50 UTC
Message-ID: <CAGZ79kaPVFT=yeSAkmf55+qXcCgMkwpgkC8jXhuZvqj5mWtgLw@mail.gmail.com>
In-Reply-To: <20160616122729.GC15988@sigill.intra.peff.net>

On Thu, Jun 16, 2016 at 5:27 AM, Jeff King <peff@peff.net> wrote:

Show 18 quoted lines

> On Wed, Jun 15, 2016 at 08:24:47AM +0200, Alex Prengère wrote:
>
>> I see, it makes sense ;-) Indeed it would seem logical to have all
>> commands showing diffs (diff, add -p, log -p, reset -p, etc..) respect
>> the diff options.
>
> Here's a patch to do so, similar to what we do for diff.algorithm.
>
> -- >8 --
> Subject: add--interactive: respect diff.compactionHeuristic
>
> We use plumbing to generate the diff, so it doesn't
> automatically pick up UI config like compactionHeuristic.
> Let's forward it on, since interactive adding is porcelain.
>
> Note that we only need to handle the "true" case. There's no
> point in passing --no-compaction-heuristic when the variable
> is false, since nothing else could have turned it on.

because we don't want to implement --[no-]compaction-heuristic as a command line switch to git-add? Fine with me.

Stepping back and looking how the compaction heuristic turned out, I think this is what we did not want to see, i.e. the need to bring it in every command, but rather enable and release it. But we backed off of the default-on, and now people may ask for the --no-compaction-heuristic in interactive add eventually, when they run into a corner case.

For now:
Reviewed-by: Stefan Beller <sbeller@google.com>

Thanks, Stefan

Show 35 quoted lines

>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  git-add--interactive.perl | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 822f857..642cce1 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -45,6 +45,7 @@ my ($diff_new_color) =
>  my $normal_color = $repo->get_color("", "reset");
>
>  my $diff_algorithm = $repo->config('diff.algorithm');
> +my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');
>  my $diff_filter = $repo->config('interactive.difffilter');
>
>  my $use_readkey = 0;
> @@ -749,6 +750,9 @@ sub parse_diff {
>         if (defined $diff_algorithm) {
>                 splice @diff_cmd, 1, 0, "--diff-algorithm=${diff_algorithm}";
>         }
> +       if ($diff_compaction_heuristic) {
> +               splice @diff_cmd, 1, 0, "--compaction-heuristic";
> +       }
>         if (defined $patch_mode_revision) {
>                 push @diff_cmd, get_diff_reference($patch_mode_revision);
>         }
> --
> 2.9.0.160.g4984cba
>
> --
> 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