Re: [PATCH v3 16/20] range-diff --dual-color: work around bogus white-space warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 3, 2018 at 4:26 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> When displaying a diff of diffs, it is possible that there is an outer
> `+` before a context line. That happens when the context changed between
> old and new commit. When that context line starts with a tab (after the
> space that marks it as context line), our diff machinery spits out a
> white-space error (space before tab), but in this case, that is
> incorrect.
>
> Work around this by detecting that situation and simply *not* printing
> the space in that case.

ok. If that is the workaround that you deem to be the right thing for now.
(I do not have an opinion if that is the right approach, or if we'd want
to s/<TAB>/<SPACE>/ for example.)

> This is slightly improper a fix because it is conceivable that an
> output_prefix might be configured with *just* the right length to let
> that tab jump to a different tab stop depending whether we emit that
> space or not.
>
> However, the proper fix would be relatively ugly and intrusive because
> it would have to *weaken* the WS_SPACE_BEFORE_TAB option in ws.c.
> Besides, we do not expose the --dual-color option in cases other than
> the `git range-diff` command, which only uses a hard-coded
> output_prefix of four spaces (which misses the problem by one
> column... ;-)).

That makes sense!

> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
>  diff.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/diff.c b/diff.c
> index 26445ffa1..325007167 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1093,6 +1093,12 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
>                                 set = diff_get_color_opt(o, DIFF_FRAGINFO);
>                         else if (c != '+')
>                                 set = diff_get_color_opt(o, DIFF_CONTEXT);
> +                       /* Avoid space-before-tab warning */
> +                       if (c == ' ' && (len < 2 || line[1] == '\t' ||
> +                                        line[1] == '\r' || line[1] == '\n')) {
> +                               line++;
> +                               len--;
> +                       }
>                 }

And this is inside the check for 'o->flags.dual_color_diffed_diffs',
so that is protected against other diffs.

Thanks,
Stefan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux