From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 22/25] diff.c: color moved lines differently
Date: Fri, 30 Jun 2017 10:54:19 -0700 [thread overview]
Message-ID: <xmqqd19ls6no.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170630000710.10601-23-sbeller@google.com> (Stefan Beller's message of "Thu, 29 Jun 2017 17:07:07 -0700")
Stefan Beller <sbeller@google.com> writes:
> +static int next_byte(const char **cp, const char **endp,
> + const struct diff_options *diffopt)
> +{
> + int retval;
> +
> + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_AT_EOL)) {
> + while (*endp > *cp && isspace(**endp))
> + (*endp)--;
> + }
This should be done by the callers (both moved_entry_cmp() and
get_string_hash()) before starting to iterate over the bytes from
the beginning, no?
> + if (*cp > *endp)
> + return -1;
> +
> + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_CHANGE)) {
> + while (*cp < *endp && isspace(**cp))
> + (*cp)++;
> + /*
> + * After skipping a couple of whitespaces, we still have to
> + * account for one space.
> + */
> + return (int)' ';
> + }
> +
> + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE)) {
> + while (*cp < *endp && isspace(**cp))
> + (*cp)++;
> + /* return the first non-ws character via the usual below */
> + }
> +
> + retval = **cp;
The char could be signed, and byte 0xff may become indistinguishable
from the EOF (i.e. -1) you returned earlier.
> + (*cp)++; /* advance */
> + return retval;
> +}
> +
> +static int moved_entry_cmp(const struct moved_entry *a,
> + const struct moved_entry *b,
> + const void *keydata,
> + const void *data)
> +{
> + const struct diff_options *diffopt = data;
> + const char *ap = a->es->line, *ae = a->es->line + a->es->len;
> + const char *bp = b->es->line, *be = b->es->line + b->es->len;
> +
> + if (!(diffopt->xdl_opts & XDF_WHITESPACE_FLAGS))
> + return a->es->len != b->es->len || memcmp(ap, bp, a->es->len);
> +
> + while (1) {
> + int ca, cb;
> + ca = next_byte(&ap, &ae, diffopt);
> + cb = next_byte(&bp, &be, diffopt);
> + if (ca != cb)
> + return 1; /* differs */
> + if (!ca)
Shouldn't this check for "ca == -1", as we are not dealing with NUL
terminated string but a <ptr, len> thing?
> + return 0;
> + };
> +}
> +
> +static unsigned get_string_hash(struct emitted_diff_symbol *es, struct diff_options *o)
> +{
> + if (o->xdl_opts & XDF_WHITESPACE_FLAGS) {
> + static struct strbuf sb = STRBUF_INIT;
> + const char *ap = es->line, *ae = es->line + es->len;
> + int c;
> +
> + strbuf_reset(&sb);
> + while ((c = next_byte(&ap, &ae, o)) > 0)
> + strbuf_addch(&sb, c);
> +
> + return memhash(sb.buf, sb.len);
> + } else {
> + return memhash(es->line, es->len);
> + }
> +}
next prev parent reply other threads:[~2017-06-30 17:54 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-30 0:06 [PATCH 00/25] Reroll of sb/diff-color-moved Stefan Beller
2017-06-30 0:06 ` [PATCH 01/25] diff.c: readability fix Stefan Beller
2017-06-30 0:06 ` [PATCH 02/25] diff.c: move line ending check into emit_hunk_header Stefan Beller
2017-06-30 0:06 ` [PATCH 03/25] diff.c: factor out diff_flush_patch_all_file_pairs Stefan Beller
2017-06-30 0:06 ` [PATCH 04/25] diff.c: introduce emit_diff_symbol Stefan Beller
2017-06-30 0:06 ` [PATCH 05/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKER Stefan Beller
2017-06-30 0:06 ` [PATCH 06/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFO Stefan Beller
2017-06-30 0:06 ` [PATCH 07/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOF Stefan Beller
2017-06-30 0:06 ` [PATCH 08/25] diff.c: migrate emit_line_checked to use emit_diff_symbol Stefan Beller
2017-06-30 0:06 ` [PATCH 09/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN] Stefan Beller
2017-06-30 0:06 ` [PATCH 10/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETE Stefan Beller
2017-06-30 0:06 ` [PATCH 11/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS} Stefan Beller
2017-06-30 0:06 ` [PATCH 12/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADER Stefan Beller
2017-06-30 0:06 ` [PATCH 13/25] diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILES Stefan Beller
2017-06-30 0:06 ` [PATCH 14/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFF Stefan Beller
2017-06-30 0:07 ` [PATCH 15/25] submodule.c: migrate diff output to use emit_diff_symbol Stefan Beller
2017-06-30 0:07 ` [PATCH 16/25] diff.c: convert emit_binary_diff_body " Stefan Beller
2017-06-30 0:07 ` [PATCH 17/25] diff.c: convert show_stats " Stefan Beller
2017-06-30 0:07 ` [PATCH 18/25] diff.c: convert word diffing " Stefan Beller
2017-06-30 0:07 ` [PATCH 19/25] diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEP Stefan Beller
2017-06-30 0:07 ` [PATCH 20/25] diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY Stefan Beller
2017-06-30 0:07 ` [PATCH 21/25] diff.c: buffer all output if asked to Stefan Beller
2017-06-30 0:07 ` [PATCH 22/25] diff.c: color moved lines differently Stefan Beller
2017-06-30 17:54 ` Junio C Hamano [this message]
2017-06-30 18:13 ` Stefan Beller
2017-06-30 18:41 ` Junio C Hamano
2017-06-30 0:07 ` [PATCH 23/25] diff.c: color moved lines differently, plain mode Stefan Beller
2017-06-30 0:07 ` [PATCH 24/25] diff.c: add dimming to moved line detection Stefan Beller
2017-06-30 0:07 ` [PATCH 25/25] diff: document the new --color-moved setting Stefan Beller
2017-06-30 7:26 ` Simon Ruderich
2017-06-30 16:04 ` Stefan Beller
2017-06-30 20:31 ` Simon Ruderich
2017-06-30 20:33 ` Stefan Beller
2017-06-30 20:52 ` [PATCHv2 00/25] Reroll of sb/diff-color-moved Stefan Beller
2017-06-30 20:52 ` [PATCHv2 01/25] diff.c: readability fix Stefan Beller
2017-06-30 20:52 ` [PATCHv2 02/25] diff.c: move line ending check into emit_hunk_header Stefan Beller
2017-06-30 20:52 ` [PATCHv2 03/25] diff.c: factor out diff_flush_patch_all_file_pairs Stefan Beller
2017-06-30 20:52 ` [PATCHv2 04/25] diff.c: introduce emit_diff_symbol Stefan Beller
2017-06-30 20:52 ` [PATCHv2 05/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKER Stefan Beller
2017-06-30 20:52 ` [PATCHv2 06/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFO Stefan Beller
2017-06-30 20:52 ` [PATCHv2 07/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOF Stefan Beller
2017-06-30 20:52 ` [PATCHv2 08/25] diff.c: migrate emit_line_checked to use emit_diff_symbol Stefan Beller
2017-06-30 20:52 ` [PATCHv2 09/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN] Stefan Beller
2017-06-30 20:52 ` [PATCHv2 10/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETE Stefan Beller
2017-06-30 20:52 ` [PATCHv2 11/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS} Stefan Beller
2017-06-30 20:52 ` [PATCHv2 12/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADER Stefan Beller
2017-06-30 20:52 ` [PATCHv2 13/25] diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILES Stefan Beller
2017-06-30 20:52 ` [PATCHv2 14/25] diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFF Stefan Beller
2017-06-30 20:53 ` [PATCHv2 15/25] submodule.c: migrate diff output to use emit_diff_symbol Stefan Beller
2017-06-30 20:53 ` [PATCHv2 16/25] diff.c: convert emit_binary_diff_body " Stefan Beller
2017-06-30 20:53 ` [PATCHv2 17/25] diff.c: convert show_stats " Stefan Beller
2017-06-30 20:53 ` [PATCHv2 18/25] diff.c: convert word diffing " Stefan Beller
2017-06-30 20:53 ` [PATCHv2 19/25] diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEP Stefan Beller
2017-06-30 20:53 ` [PATCHv2 20/25] diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY Stefan Beller
2017-06-30 20:53 ` [PATCHv2 21/25] diff.c: buffer all output if asked to Stefan Beller
2017-06-30 20:53 ` [PATCHv2 22/25] diff.c: color moved lines differently Stefan Beller
2017-06-30 21:11 ` Junio C Hamano
2017-06-30 21:21 ` Stefan Beller
2017-06-30 21:36 ` Junio C Hamano
2017-06-30 20:53 ` [PATCHv2 23/25] diff.c: color moved lines differently, plain mode Stefan Beller
2017-06-30 20:53 ` [PATCHv2 24/25] diff.c: add dimming to moved line detection Stefan Beller
2017-06-30 20:53 ` [PATCHv2 25/25] diff: document the new --color-moved setting Stefan Beller
2017-06-30 21:49 ` [PATCHv2 00/25] Reroll of sb/diff-color-moved 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=xmqqd19ls6no.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sbeller@google.com \
/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).