All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, git@drmicha.warpmail.net
Subject: Re: [PATCH 03/16] list-files: show paths relative to cwd
Date: Thu, 12 Mar 2015 14:28:05 -0700	[thread overview]
Message-ID: <xmqqoanx29ei.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqsid929rb.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Thu, 12 Mar 2015 14:20:24 -0700")

Junio C Hamano <gitster@pobox.com> 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.

  reply	other threads:[~2015-03-12 21:28 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 10:18 [PATCH 00/16] nd/list-files redesign Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 01/16] list-files: command skeleton Nguyễn Thái Ngọc Duy
2015-03-12 21:02   ` Junio C Hamano
2015-03-09 10:18 ` [PATCH 02/16] list-files: make :(glob) pathspec default Nguyễn Thái Ngọc Duy
2015-03-12 21:10   ` Junio C Hamano
2015-03-14 11:21     ` Duy Nguyen
2015-03-09 10:18 ` [PATCH 03/16] list-files: show paths relative to cwd Nguyễn Thái Ngọc Duy
2015-03-12 21:20   ` Junio C Hamano
2015-03-12 21:28     ` Junio C Hamano [this message]
2015-03-14 11:25       ` Duy Nguyen
2015-03-15 21:16         ` Junio C Hamano
2015-03-15 23:41           ` Duy Nguyen
2015-03-09 10:18 ` [PATCH 04/16] list-files: add tag to each entry, filter duplicate tags Nguyễn Thái Ngọc Duy
2015-03-12 21:48   ` Junio C Hamano
2015-03-09 10:18 ` [PATCH 05/16] list-files: add --[no-]column, -C and -1 Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 06/16] list-files: add --max-depth and -R Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 07/16] list-files: show directories as well as files Nguyễn Thái Ngọc Duy
2015-03-10  6:23   ` Eric Sunshine
2015-03-10  6:39     ` Duy Nguyen
2015-03-09 10:18 ` [PATCH 08/16] list-files: add --color Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 09/16] list-files: add -F/--classify Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 10/16] list-files: new indicator '&' for submodules when -F is used Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 11/16] list-files: add --cached and --others Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 12/16] list-files: add --ignored Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 13/16] list-files: add --unmerged Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 14/16] list-files: add file modification options -[admADM] Nguyễn Thái Ngọc Duy
2015-03-09 10:18 ` [PATCH 15/16] list-files: delete redundant cached entries Nguyễn Thái Ngọc Duy
2015-03-10  6:28   ` Eric Sunshine
2015-03-09 10:18 ` [PATCH 16/16] list-files: make alias 'ls' default to 'list-files' Nguyễn Thái Ngọc Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqoanx29ei.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.