Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable
- From
Ping Yin <pkufranky@gmail.com>
- Date
- May 10, 2008, 08:20 UTC
- Message-ID
- <46dff0320805100120xe967359v62d665fdbaca5e6a@mail.gmail.com>
- In-Reply-To
- <7viqxqc4gs.fsf@gitster.siamese.dyndns.org>
On Thu, May 8, 2008 at 3:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 93 quoted lines
> I haven't tested this at all (this is a lunchtime hack) and have a mild
> suspicion that it may have corner case miscounting (e.g. I blindly
> subtracts 3 from len when dealing with a line that represents a single
> token from the internal diff output --- do I always have 3 there even when
> the original file ends with an incomplete line? I didn't check), but
> other than that I think this is a lot easier to read and follow.
>
> ---
>
> diff.c | 216 +++++++++++++++++++++++++++++++--------------------------------
> 1 files changed, 106 insertions(+), 110 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index e35384b..344aaa6 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -351,87 +351,119 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
> return 0;
> }
>
> -struct diff_words_buffer {
> - mmfile_t text;
> - long alloc;
> - long current; /* output pointer */
> - int suppressed_newline;
> +typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
> +
> +struct emit_callback {
> + struct xdiff_emit_state xm;
> + int nparents, color_diff;
> + unsigned ws_rule;
> + sane_truncate_fn truncate;
> + const char **label_path;
> + struct diff_words_data *diff_words;
> + int *found_changesp;
> + FILE *file;
> };
>
> -static void diff_words_append(char *line, unsigned long len,
> - struct diff_words_buffer *buffer)
> +static size_t diff_words_tokenize(struct emit_callback *ecbdata,
> + char *line, unsigned long len)
> {
> - if (buffer->text.size + len > buffer->alloc) {
> - buffer->alloc = (buffer->text.size + len) * 3 / 2;
> - buffer->text.ptr = xrealloc(buffer->text.ptr, buffer->alloc);
> + /*
> + * This function currently is deliberately done very stupid,
> + * but passing ecbdata here means that you can potentially
> + * implement different tokenization rules depending on
> + * the content (e.g. "gitattributes(5)").
> + */
> + int is_space;
> + char *line0 = line;
> +
> + if (!len)
> + return 0;
> +
> + is_space = isspace(*line);
> + while (len && (isspace(*line) == is_space)) {
> + line++;
> + len--;
> }
> + return line - line0;
> +}
> +
> +static void diff_words_append(struct emit_callback *ecbdata,
> + char *line, unsigned long len,
> + struct strbuf *text)
> +{
> + /* Skip leading +/- first. */
> line++;
> len--;
> - memcpy(buffer->text.ptr + buffer->text.size, line, len);
> - buffer->text.size += len;
> +
> + /*
> + * Tokenize and stuff the words in.
> + */
> + while (len) {
> + size_t token_len = diff_words_tokenize(ecbdata, line, len);
> +
> + if (line[0] != '\n') {
> + /*
> + * A nonempty token has ' ' stuffed in front,
> + * so that we can recover the original
> + * end-of-line easily. Stupid, but works.
> + */
> + strbuf_add(text, " ", 1);
> + strbuf_add(text, line, token_len);
> + strbuf_add(text, "\n", 1);
> + len -= token_len;
> + line += token_len;I still don't understand why a ' ' is prepended. See my comment for the following part
Show 9 quoted lines
> + if (line[1] == ' ') {
> + /* A token */
> + line += 2;
> + len -= 3; /* drop the trailing LF */
> + } else {
> + /* A real LF */
> + line++;
> + len--;
> }I think we can recognize a real LF by that the diff line should be a single '\n', i.e. line[1] == '\n'. So what's wrong by s/line[1] == ' '/line[1] != '\n'/ ?
-- Ping Yin