Re: [PATCH 03/16] list-files: show paths relative to cwd

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 12, 2015, 21:20 UTC
Message-ID: <xmqqsid929rb.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1425896314-10941-4-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 26 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/list-files.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/list-files.c b/builtin/list-files.c
> index b99f2b7..c444a53 100644
> --- a/builtin/list-files.c
> +++ b/builtin/list-files.c
> @@ -3,6 +3,7 @@
>  #include "parse-options.h"
>  #include "pathspec.h"
>  #include "dir.h"
> +#include "quote.h"
>  
>  static struct pathspec pathspec;
>  static const char *prefix;
> @@ -22,7 +23,7 @@ static void add_one(struct string_list *result, const char *name)
>  	struct strbuf sb = STRBUF_INIT;
>  	struct string_list_item *item;
>  
> -	strbuf_addstr(&sb, name);
> +	quote_path_relative(name, prefix_length ? prefix : NULL, &sb);
>  	item = string_list_append(result, strbuf_detach(&sb, NULL));
>  	item->util = (char *)name;
>  }

Hmph, wouldn't it make it more cumbersome and problematic to do things like this here in add_one() phase? I am imagining that the endgame of this program will be

    - populate_cached_entries() reads from the data source (at this
      step, there is just "the index"), calling add_one() whose
      responsibility is to record the paths that are interesting
      to an in-core structure;

    - perform some interesting filtering, annotating, ordering,
      etc. (at this step, there is none) yet to come;

    - display() will iterate over the result and then format the
      result.

For example, if you do the "quote" thing too early, don't codepaths in the later phases have to worry about item->string not matching the original pathname anymore? If you want to do something like "/bin/ls -t", you may have to lstat() the paths for each item, but if these store a path relative to the prefix, wouldn't you have to prepend the prefix again before running lstat()?

I am just wondering if this prefix-stripping and quoting belongs to the output phase, not the input phase.