git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Ping YinNext: Johannes Schindelin
Message 49 of 92 in “Make words boundary for --color-words configurable”
  1. Make words boundary for --color-words configurablePing Yin, May 2, 2008
  2. Junio C HamanoMay 2, 2008
  3. Ping YinMay 2, 2008
  4. Make boundary characters for --color-words configurablePing Yin, May 2, 2008
  5. Ping YinMay 2, 2008
  6. Ping YinMay 2, 2008
  7. 0/5 Make boundary characters for --color-words configurablePing Yin, May 3, 2008
  8. 1/5 diff.c: Remove code redundancy in diff_words_showPing Yin, May 3, 2008
  9. 2/5 diff.c: Use show variable name in fn_out_diff_words_auxPing Yin, May 3, 2008
  10. 3/5 diff.c: Fix --color-words showing trailing deleted words at another linePing Yin, May 3, 2008
  11. 4/5 Make boundary characters for --color-words configurablePing Yin, May 3, 2008
  12. 5/5 fn_out_diff_words_aux: Handle common diff line more carefullyPing Yin, May 3, 2008
  13. Junio C HamanoMay 3, 2008
  14. Teemu LikonenMay 3, 2008
  15. Ping YinMay 4, 2008
  16. Johannes SchindelinMay 4, 2008
  17. Ping YinMay 4, 2008
  18. Junio C HamanoMay 4, 2008
  19. Jakub NarebskiMay 4, 2008
  20. Teemu LikonenMay 4, 2008
  21. Johannes SchindelinMay 5, 2008
  22. Ping YinMay 5, 2008
  23. Junio C HamanoMay 5, 2008
  24. Ping YinMay 5, 2008
  25. Ping YinMay 6, 2008
  26. Johannes SchindelinMay 6, 2008
  27. Ping YinMay 7, 2008
  28. Johannes SchindelinMay 7, 2008
  29. Ping YinMay 7, 2008
  30. Johannes SchindelinMay 7, 2008
  31. Ping YinMay 7, 2008
  32. Junio C HamanoMay 7, 2008
  33. Junio C HamanoMay 7, 2008
  34. Jeff KingMay 7, 2008
  35. Junio C HamanoMay 7, 2008
  36. Jeff KingMay 7, 2008
  37. Teemu LikonenMay 8, 2008
  38. Ping YinMay 10, 2008
  39. Teemu LikonenMay 10, 2008
  40. Ping YinMay 11, 2008
  41. Ping YinMay 11, 2008
  42. Junio C HamanoMay 11, 2008
  43. Ping YinMay 12, 2008
  44. Jakub NarebskiMay 12, 2008
  45. Junio C HamanoMay 12, 2008
  46. Jakub NarebskiMay 12, 2008
  47. Ping YinMay 13, 2008
  48. Ping YinMay 13, 2008
  49. Ping YinMay 10, 2008
  50. Johannes SchindelinMay 5, 2008
  51. Ping YinMay 5, 2008
  52. Junio C HamanoMay 3, 2008
  53. Ping YinMay 3, 2008
  54. Junio C HamanoMay 3, 2008
  55. Junio C HamanoMay 3, 2008
  56. 0/6 --color-words improvementPing Yin, May 4, 2008
  57. 1/6 diff.c: Remove code redundancy in diff_words_showPing Yin, May 4, 2008
  58. 2/6 fn_out_diff_words_aux: Use short variable namePing Yin, May 4, 2008
  59. 3/6 --color-words: Fix showing trailing deleted words at another linePing Yin, May 4, 2008
  60. 4/6 --color-words: Make non-word characters configurablePing Yin, May 4, 2008
  61. 5/6 fn_out_diff_words_aux: Handle common diff line more carefullyPing Yin, May 4, 2008
  62. 6/6 --color-words: Add test t4030Ping Yin, May 4, 2008
  63. Johannes SchindelinMay 4, 2008
  64. Ping YinMay 4, 2008
  65. Johannes SchindelinMay 5, 2008
  66. Ping YinMay 5, 2008
  67. Junio C HamanoMay 4, 2008
  68. Ping YinMay 4, 2008
  69. Johannes SchindelinMay 4, 2008
  70. Ping YinMay 4, 2008
  71. Johannes SchindelinMay 5, 2008
  72. Johannes SchindelinMay 4, 2008
  73. Ping YinMay 4, 2008
  74. Johannes SchindelinMay 5, 2008
  75. Johannes SchindelinMay 4, 2008
  76. Teemu LikonenMay 2, 2008
  77. Ping YinMay 3, 2008
  78. Dirk SüsserottMay 3, 2008
  79. Ping YinMay 3, 2008
  80. --color-words: Make the word characters configurableJohannes Schindelin, May 3, 2008
  81. Ping YinMay 3, 2008
  82. Johannes SchindelinMay 3, 2008
  83. Teemu LikonenMay 3, 2008
  84. Johannes SchindelinMay 4, 2008
  85. Junio C HamanoMay 3, 2008
  86. Johannes SchindelinMay 4, 2008
  87. Johannes SchindelinMay 2, 2008
  88. Teemu LikonenMay 2, 2008
  89. Ping YinMay 2, 2008
  90. Teemu LikonenMay 2, 2008
  91. Ping YinMay 2, 2008
  92. Jakub NarebskiMay 3, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.