Re: [PATCH alt-v1] Improve function dir.c:trim_trailing_spaces()

> Discard unnecessary 'nr_spaces' variable and improve
> the 'if' structure. Switch to pointers instead of integers
>
> Slightly more rare occurrences of 'text  \    ' with a backslash
> in between spaces are handled correctly. Namely, the code in
> 8ba87adad6 does not reset 'last_space' when a backslash is
> encountered and the above line stays intact as
> a result. Add a test at the end of t/t0008-ignores.sh to
> exhibit this behavior

>
> Signed-off-by: Pasha Bolokhov <pasha.bolokhov@gmail.com>
> ---
> Instead of the "optimized" version, which had complaints about
> reasonability, I am attaching the "correction" of the original version.
> 'last_space' was not reset after encountering a backslash, and
> 'nr_spaces' is an unnecessary variable.
>
> I myself am still leaning towards the optimized version (which scans
> backwards instead of forward), since it completely ignores strings
> which do not have spaces at the end, while these forward implementations
> (both the one being attached and the original one) always scan
> all strings through. Again, I'm not attaching the optimized one
> because its readability is indeed less
>
>  dir.c              | 29 ++++++++++++++---------------
>  t/t0008-ignores.sh | 21 +++++++++++++++++++++
>  2 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index eb6f581..e67dcc2 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -508,21 +508,20 @@ void clear_exclude_list(struct exclude_list *el)
>
>  static void trim_trailing_spaces(char *buf)
>  {
> -       int i, last_space = -1, nr_spaces, len = strlen(buf);
> -       for (i = 0; i < len; i++)
> -               if (buf[i] == '\\')
> -                       i++;
> -               else if (buf[i] == ' ') {
> -                       if (last_space == -1) {
> -                               last_space = i;
> -                               nr_spaces = 1;
> -                       } else
> -                               nr_spaces++;
> -               } else
> -                       last_space = -1;
> -
> -       if (last_space != -1 && last_space + nr_spaces == len)
> -               buf[last_space] = '\0';
> +       char *p, *last_space = NULL;
> +
> +       for (p = buf; *p; p++)
> +               if (*p == ' ') {
> +                       if (last_space == NULL)
> +                               last_space = p;
> +               } else {
> +                       if (*p == '\\')
> +                               p++;
> +                       last_space = NULL;
> +               }
> +
> +       if (last_space != NULL)
> +               *last_space = '\0';
>  }
>
>  int add_excludes_from_file_to_list(const char *fname,
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index 63beb99..7becf96 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -806,4 +806,25 @@ test_expect_success !MINGW 'quoting allows trailing whitespace' '
>         test_cmp err.expect err
>  '
>
> +test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
> +       rm -rf whitespace &&
> +       mkdir whitespace &&
> +       >"whitespace/trailing 1  "      &&
> +       >"whitespace/trailing 2 \\\\"   &&
> +       >"whitespace/trailing 3 \\\\"   &&
> +       >"whitespace/trailing 4   \\ "  &&
> +       >"whitespace/trailing 5 \\ \\ " &&
> +       >whitespace/untracked &&
> +       echo "whitespace/trailing 1 \\    "                     >ignore  &&
> +       echo "whitespace/trailing 2 \\\\\\\\\\\\\\\\"           >>ignore &&
> +       echo "whitespace/trailing 3 \\\\\\\\\\\\\\\\ "          >>ignore &&
> +       echo "whitespace/trailing 4   \\\\\\\\\\\\    "         >>ignore &&
> +       echo "whitespace/trailing 5 \\\\\\\\ \\\\\\\\\\\\   "   >>ignore &&
> +       echo whitespace/untracked >expect &&
> +       : >err.expect &&
> +       git ls-files -o -X ignore whitespace >actual 2>err &&
> +       test_cmp expect actual &&
> +       test_cmp err.expect err
> +'
> +
>  test_done
> --
> 1.9.1
>