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

From: Junio C Hamano <[email-masked]>
Date: Thu, 12 Mar 2015 14:28:05 -0700

Junio C Hamano <[email-masked]> writes:

>>  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.

Hmph, another interpretation of this patch is that your item->string
are not the true filenames but the result of applying some
interesting processing to the filenames and the true filenames are
kept in item->util.  Is that what is going on?

If that is the case, it sort of makes sense to me, even though it
would feel a bit unusual way to use the string-list.

Thanks.