Re: [PATCH REPLACEMENT for 2/2] git status: show relative paths when run in a subdirectory

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

please, please, please do not cull the Cc list.  I consider it rude to 
reply to _me_, but _address_ the mail to me, either the To: (preferred) or 
the Cc: (not so preferred).

On Sat, 10 Nov 2007, Michel Marti wrote:

> Untracked files in the current dir don't include the relative path 
> to the project-root, but changed/updated files do:
> 
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       new file: ../subdir/hello
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       world
> 
> With the patch below (on top of your changes), the output becomes
> 
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       new file: hello
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       world
> 
> Cheers,
> 
> - Michel
> 
> diff --git a/wt-status.c b/wt-status.c
> index 0d25362..2cdc8ce 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -133,8 +133,8 @@ static void wt_status_print_filepair(struct wt_status *s,
>  
>         strbuf_init(&onebuf, 0);
>         strbuf_init(&twobuf, 0);
> -       one = quote_path(p->one->path, -1, &onebuf, s->prefix);
> -       two = quote_path(p->two->path, -1, &twobuf, s->prefix);
> +       one = quote_path(p->one->path, strlen(p->one->path), &onebuf, s->prefix);
> +       two = quote_path(p->two->path, strlen(p->two->path), &twobuf, s->prefix);
>  
>         color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
>         switch (p->status) {
> @@ -233,7 +233,8 @@ static void wt_status_print_initial(struct wt_status *s)
>         for (i = 0; i < active_nr; i++) {
>                 color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
>                 color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
> -                               quote_path(active_cache[i]->name, -1,
> +                               quote_path(active_cache[i]->name,
> +                                       strlen(active_cache[i]->name),
>                                            &buf, s->prefix));
>         }
>         if (active_nr)
> 

This patch is wrong.

If you want to go that way, move the strlen() call _into_ quote_path(), 
like I had it earlier.

But then we will have a double traversal of the strings again.  That's 
what I tried to avoid, but I missed one place:

In line 94, it says "... && off < len && ...".  This should read something 
like "((len < 0 && !in[off]) || off < len)" instead.  Or maybe even "(len 
< 0 || off < len)" and have an "} else if (in[off]) off++; else break;" in 
the loop block.

Besides, you completely ignored the nice examples how other people 
contribute their patches, with mail bodies that double as a commit 
message, a diffstat, and with a test case.

Hth,
Dscho