From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-11.4 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,T_DKIMWL_WL_MED,USER_IN_DEF_DKIM_WL shortcircuit=no autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id DC7C01F85B for ; Tue, 10 Jul 2018 21:54:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732285AbeGJVzs (ORCPT ); Tue, 10 Jul 2018 17:55:48 -0400 Received: from mail-qt0-f201.google.com ([209.85.216.201]:49786 "EHLO mail-qt0-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732274AbeGJVzs (ORCPT ); Tue, 10 Jul 2018 17:55:48 -0400 Received: by mail-qt0-f201.google.com with SMTP id b8-v6so22443121qto.16 for ; Tue, 10 Jul 2018 14:54:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:date:in-reply-to:message-id:references:subject:from:to :cc; bh=KpBfc8futXpj0osbqTlPOvZF+kE88GiTJ5zE6TFYiWo=; b=R1pJnLJX27bolCekgIONBuUWLToQgyILEnSI5QhKpxMrUlXQs0fXTzSvKgxGY0eF46 qK2yrEotBbEAfcMO5SHOgYzFmIHbk9MlS7ZX4F1vCrY1uwsIfVAqCzUHAKT2DWFBSZTG f+ZC9BN0K+Dsr1dvNK9JUV64yazWCHff1WkfMINjUV70rlsfOr9DJIrPpNdKhpMA52iQ GjBxJgGn5jSORk59DXm+q/K68rZM/SMZgCH3JRExxBOcjBOjT5kIA/56yO4udpy4rOqr rWfL7/X221gLrYTEYs0ccXW0kw4TugJwA14qmau2d+kcR4ONBNT7MayflV8anmV/26SM 0UCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:date:in-reply-to:message-id :references:subject:from:to:cc; bh=KpBfc8futXpj0osbqTlPOvZF+kE88GiTJ5zE6TFYiWo=; b=UtMRB5qOl1L9iuGoXHoSap9MgNE2vTNnSIulvGFzNV9g1Xmb2Z8sH0gXRMa66tt/l+ /nu7ROfDuZ9JA5ImmyO/jixhZZZW76dz+X2Zid98jNwfzQSWOf+12HnMxGndKn3zIx7L gVyxvK4rtZw1PS28HlANmhyGNq4pNKvEWrolvRxsBHqlpHTdXqCW4HGq4cdbXaCcApQE +pikYJR0D0TwIvH/agx/PhdsjptpbY+LBMog1QG5GClvsAqCUBFbHZSLaQWNscxzf2zG dVFS6J2g9gFTN/Dx3VTkrl2GMGKG06JpmsY2oRvRnLDvda4jYJRAyKzJszpGSnJv8/xs KiHQ== X-Gm-Message-State: APt69E2Ru+D9qIPl333bvoI+XU4CF679HR7nl98GVqKBtf5TrCJ9/eYw daRzsczNFwvKb1fVoAO+ocxptc+kDbRb X-Google-Smtp-Source: AAOMgpcxEwjWVNVX28CtueqZSPHT1mTIpSWxLq3rf8K8aGOSYNYBlCYHSMZobxqS0gv+2di8sRzyyWbYqom2 MIME-Version: 1.0 X-Received: by 2002:ac8:2758:: with SMTP id h24-v6mr15458518qth.39.1531259683695; Tue, 10 Jul 2018 14:54:43 -0700 (PDT) Date: Tue, 10 Jul 2018 14:54:37 -0700 In-Reply-To: <20180710195921.131548-1-sbeller@google.com> Message-Id: <20180710215437.33904-1-sbeller@google.com> References: <20180710195921.131548-1-sbeller@google.com> X-Mailer: git-send-email 2.18.0.203.gfac676dfb9-goog Subject: [PATCH] ws: do not reset and set color twice From: Stefan Beller To: sbeller@google.com Cc: git@vger.kernel.org, gitgitgadget@gmail.com, gitster@pobox.com, johannes.schindelin@gmx.de Content-Type: text/plain; charset="UTF-8" Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When outputting lines that are checked for white space, we first use emit_line_0 to emit the prefix, and then the ws specific code. The code at each site carefully sets the color and then resets it, though it is the same color. Avoid setting the color twice by passing a newly introduced flag that indicates if the color is already set. Signed-off-by: Stefan Beller --- The whole series is also available via git fetch http://github.com/stefanbeller/git ws_cleanup-ontop-range-diff and concludes my cleanup on top of the range-diff series for now. What is left is to refactor the diff-diff from the range-diff series to utilize the marker at the beginning of the line that can be more than one character. I left that as I do not want to collide with your work. Thanks, Stefan cache.h | 2 +- diff.c | 8 +-- t/t4015-diff-whitespace.sh | 122 ++++++++++++++++++------------------- ws.c | 28 +++++++-- 4 files changed, 89 insertions(+), 71 deletions(-) diff --git a/cache.h b/cache.h index d49092d94d1..8f53a65fa36 100644 --- a/cache.h +++ b/cache.h @@ -1807,7 +1807,7 @@ extern unsigned whitespace_rule_cfg; extern unsigned whitespace_rule(const char *); extern unsigned parse_whitespace_rule(const char *); extern unsigned ws_check(const char *line, int len, unsigned ws_rule); -extern void ws_check_emit(const char *line, int len, unsigned ws_rule, FILE *stream, const char *set, const char *reset, const char *ws); +extern void ws_check_emit(const char *line, int len, unsigned ws_rule, FILE *stream, const char *set, const char *reset, const char *ws, int already_set); extern char *whitespace_error_string(unsigned ws); extern void ws_fix_copy(struct strbuf *, const char *, int, unsigned, int *); extern int ws_blank_line(const char *line, int len, unsigned ws_rule); diff --git a/diff.c b/diff.c index 0b00df7b3c8..34d02f4095b 100644 --- a/diff.c +++ b/diff.c @@ -993,11 +993,11 @@ static void emit_line_ws_markup(struct diff_options *o, /* Blank line at EOF - paint '+' as well */ emit_line_0(o, ws, set_sign, reset, sign, line, len); else { - /* Emit just the prefix, then the rest. */ - emit_line_0(o, set, set_sign, reset, + /* Emit just the prefix (with no RESET), then the rest. */ + emit_line_0(o, set, set_sign, NULL, sign, "", 0); ws_check_emit(line, len, ws_rule, - o->file, set, reset, ws); + o->file, set, reset, ws, 1); } } @@ -2918,7 +2918,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) free(err); emit_line(data->o, set, reset, line, 1); ws_check_emit(line + 1, len - 1, data->ws_rule, - data->o->file, set, reset, ws); + data->o->file, set, reset, ws, 0); } else if (line[0] == ' ') { data->lineno++; } else if (line[0] == '@') { diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 95baf237a83..8834f2040c0 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -874,9 +874,9 @@ test_expect_success 'diff that introduces a line with only tabs' ' +++ b/x @@ -1 +1,4 @@ test - +{ + +{ + - +} + +} EOF test_cmp expected current @@ -906,8 +906,8 @@ test_expect_success 'diff that introduces and removes ws breakages' ' @@ -1,2 +1,3 @@ 0. blank-at-eol -1. blank-at-eol - +1. still-blank-at-eol - +2. and a new line + +1. still-blank-at-eol + +2. and a new line EOF test_cmp expected current @@ -934,9 +934,9 @@ test_expect_success 'ws-error-highlight test setup' ' +++ b/x @@ -1,2 +1,3 @@ 0. blank-at-eol - -1. blank-at-eol - +1. still-blank-at-eol - +2. and a new line + -1. blank-at-eol + +1. still-blank-at-eol + +2. and a new line EOF cat >expect.all <<-\EOF && @@ -946,9 +946,9 @@ test_expect_success 'ws-error-highlight test setup' ' +++ b/x @@ -1,2 +1,3 @@ 0. blank-at-eol - -1. blank-at-eol - +1. still-blank-at-eol - +2. and a new line + -1. blank-at-eol + +1. still-blank-at-eol + +2. and a new line EOF cat >expect.none <<-\EOF @@ -1038,11 +1038,11 @@ test_expect_success 'detect moved code, complete file' ' --- /dev/null +++ b/main.c @@ -0,0 +1,5 @@ - +#include - +main() - +{ - +printf("Hello World"); - +} + +#include + +main() + +{ + +printf("Hello World"); + +} diff --git a/test.c b/test.c deleted file mode 100644 index a986c57..0000000 @@ -1159,12 +1159,12 @@ test_expect_success 'detect malicious moved code, inside file' ' printf("Hello World, but different\n"); } - +int secure_foo(struct user *u) - +{ - +foo(u); - +if (!u->is_allowed_foo) - +return; - +} + +int secure_foo(struct user *u) + +{ + +foo(u); + +if (!u->is_allowed_foo) + +return; + +} + int another_function() { @@ -1208,12 +1208,12 @@ test_expect_success 'plain moved code, inside file' ' printf("Hello World, but different\n"); } - +int secure_foo(struct user *u) - +{ - +foo(u); - +if (!u->is_allowed_foo) - +return; - +} + +int secure_foo(struct user *u) + +{ + +foo(u); + +if (!u->is_allowed_foo) + +return; + +} + int another_function() { @@ -1288,12 +1288,12 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' ' line 7 line 8 line 9 - +long line 1 - +long line 2 - +long line 3 - +long line 14 - +long line 15 - +long line 16 + +long line 1 + +long line 2 + +long line 3 + +long line 14 + +long line 15 + +long line 16 line 10 line 11 line 12 @@ -1332,12 +1332,12 @@ test_expect_success 'cmd option assumes configured colored-moved' ' line 7 line 8 line 9 - +long line 1 - +long line 2 - +long line 3 - +long line 14 - +long line 15 - +long line 16 + +long line 1 + +long line 2 + +long line 3 + +long line 14 + +long line 15 + +long line 16 line 10 line 11 line 12 @@ -1467,10 +1467,10 @@ test_expect_success 'move detection ignoring whitespace changes' ' --- a/lines.txt +++ b/lines.txt @@ -1,9 +1,9 @@ - +long line 6 - +long line 7 - +long line 8 - +long li ne 9 + +long line 6 + +long line 7 + +long line 8 + +long li ne 9 line 1 line 2 line 3 @@ -1491,10 +1491,10 @@ test_expect_success 'move detection ignoring whitespace changes' ' --- a/lines.txt +++ b/lines.txt @@ -1,9 +1,9 @@ - +long line 6 - +long line 7 - +long line 8 - +long li ne 9 + +long line 6 + +long line 7 + +long line 8 + +long li ne 9 line 1 line 2 line 3 @@ -1534,10 +1534,10 @@ test_expect_success 'move detection ignoring whitespace at eol' ' --- a/lines.txt +++ b/lines.txt @@ -1,9 +1,9 @@ - +long line 6 - +long line 7 - +long line 8 - +long line 9 + +long line 6 + +long line 7 + +long line 8 + +long line 9 line 1 line 2 line 3 @@ -1558,10 +1558,10 @@ test_expect_success 'move detection ignoring whitespace at eol' ' --- a/lines.txt +++ b/lines.txt @@ -1,9 +1,9 @@ - +long line 6 - +long line 7 - +long line 8 - +long line 9 + +long line 6 + +long line 7 + +long line 8 + +long line 9 line 1 line 2 line 3 @@ -1605,7 +1605,7 @@ test_expect_success '--color-moved block at end of diff output respects MIN_ALNU --- a/bar +++ b/bar @@ -0,0 +1 @@ - +line1 + +line1 diff --git a/foo b/foo --- a/foo +++ b/foo @@ -1644,8 +1644,8 @@ test_expect_success '--color-moved respects MIN_ALNUM_COUNT' ' --- a/bar +++ b/bar @@ -0,0 +1,2 @@ - +twenty chars 234567890 - +nineteen chars 456789 + +twenty chars 234567890 + +nineteen chars 456789 diff --git a/foo b/foo --- a/foo +++ b/foo @@ -1685,9 +1685,9 @@ test_expect_success '--color-moved treats adjacent blocks as separate for MIN_AL --- a/bar +++ b/bar @@ -0,0 +1,3 @@ - +7charsB - +7charsC - +7charsA + +7charsB + +7charsC + +7charsA diff --git a/foo b/foo --- a/foo +++ b/foo diff --git a/ws.c b/ws.c index a07caedd5a5..cb4a95c25dc 100644 --- a/ws.c +++ b/ws.c @@ -139,10 +139,19 @@ char *whitespace_error_string(unsigned ws) return strbuf_detach(&err, NULL); } +static inline void optional_reset(FILE *stream, const char *reset, int *already_set) +{ + if (*already_set) { + fputs(reset, stream); + *already_set = 0; + } +} + /* If stream is non-NULL, emits the line after checking. */ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, FILE *stream, const char *set, - const char *reset, const char *ws) + const char *reset, const char *ws, + int already_set) { unsigned result = 0; int written = 0; @@ -186,6 +195,7 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, if ((ws_rule & WS_SPACE_BEFORE_TAB) && written < i) { result |= WS_SPACE_BEFORE_TAB; if (stream) { + optional_reset(stream, reset, &already_set); fputs(ws, stream); fwrite(line + written, i - written, 1, stream); fputs(reset, stream); @@ -194,12 +204,14 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, } else if (ws_rule & WS_TAB_IN_INDENT) { result |= WS_TAB_IN_INDENT; if (stream) { + optional_reset(stream, reset, &already_set); fwrite(line + written, i - written, 1, stream); fputs(ws, stream); fwrite(line + i, 1, 1, stream); fputs(reset, stream); } } else if (stream) { + optional_reset(stream, reset, &already_set); fwrite(line + written, i - written + 1, 1, stream); } written = i + 1; @@ -209,6 +221,7 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, if ((ws_rule & WS_INDENT_WITH_NON_TAB) && i - written >= ws_tab_width(ws_rule)) { result |= WS_INDENT_WITH_NON_TAB; if (stream) { + optional_reset(stream, reset, &already_set); fputs(ws, stream); fwrite(line + written, i - written, 1, stream); fputs(reset, stream); @@ -224,19 +237,24 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, /* Emit non-highlighted (middle) segment. */ if (trailing_whitespace - written > 0) { - fputs(set, stream); + if (!already_set) + fputs(set, stream); fwrite(line + written, trailing_whitespace - written, 1, stream); fputs(reset, stream); + already_set = 0; } /* Highlight errors in trailing whitespace. */ if (trailing_whitespace != len) { + optional_reset(stream, reset, &already_set); fputs(ws, stream); fwrite(line + trailing_whitespace, len - trailing_whitespace, 1, stream); fputs(reset, stream); } + if (already_set) + fputs(reset, stream); if (trailing_carriage_return) fputc('\r', stream); if (trailing_newline) @@ -247,14 +265,14 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, void ws_check_emit(const char *line, int len, unsigned ws_rule, FILE *stream, const char *set, - const char *reset, const char *ws) + const char *reset, const char *ws, int already_set) { - (void)ws_check_emit_1(line, len, ws_rule, stream, set, reset, ws); + (void)ws_check_emit_1(line, len, ws_rule, stream, set, reset, ws, already_set); } unsigned ws_check(const char *line, int len, unsigned ws_rule) { - return ws_check_emit_1(line, len, ws_rule, NULL, NULL, NULL, NULL); + return ws_check_emit_1(line, len, ws_rule, NULL, NULL, NULL, NULL, 0); } int ws_blank_line(const char *line, int len, unsigned ws_rule) -- 2.18.0.203.gfac676dfb9-goog