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:20:24 -0700	[thread overview]
Message-ID: <xmqqsid929rb.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1425896314-10941-4-git-send-email-pclouds@gmail.com> ("Nguyễn	Thái Ngọc Duy"'s message of "Mon, 9 Mar 2015 17:18:21 +0700")

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

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

  reply	other threads:[~2015-03-12 21:20 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 [this message]
2015-03-12 21:28     ` Junio C Hamano
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=xmqqsid929rb.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.