[PATCH 00/16] nd/list-files redesign

[PATCH 00/16] nd/list-files redesign

[Nguyễn Thái Ngọc Duy]

On Tue, Feb 24, 2015 at 2:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> status, status -s and the like are in an ordinary user's tool box.
>> ls-files isn't, at least not with "-t", which we even mark as deprecated.
>>
>> That makes me wonder, though, how difficult it would be to
>> wt_status_collect_unchanged() and to leverage the status machinery
>> rather than ls-files.
>
> Good point.  wt-status feels like a much better infrastructure to
> build on than "ls-files -t", which should die ;-).  Especially if
> the command is interested in showing the state of the working tree
> files relative to the tree of HEAD, as "ls-files" is purely between
> the index and the working tree.

So here it is. Except the first 4 patches identical with
'nd/list-files' in 'pu' and not reposted here, the rest is rewritten
using wt-status.c as the base.  Documentation and tests are missing.
Probably bugs lurking too. I just want to check if the UI makes sense.
Full series is available at

    https://github.com/pclouds/git/commits/ls

if you want to pull and try out quickly. With this we got:

usage: git list-files [options] [<pathspec>...]

Filter options
    -c, --cached          show cached files (default)
    -o, --others          show untracked files
    -i, --ignored         show ignored files
    -u, --unmerged        show unmerged files
    -a, --added           show added files compared to HEAD
    -d, --deleted         show deleted files compared to HEAD
    -m, --modified        show modified files compared to HEAD
    -A, --wt-added        show added files in worktree
    -D, --wt-deleted      show deleted files in worktree
    -M, --wt-modified     show modified files on worktree

Other
    -C, --column[=<style>]
                          show files in columns
    -1                    shortcut for --no-column
    --max-depth <depth>   descend at most <depth> levels
    -R, --recursive       shortcut for --max-depth=-1
    --color[=<when>]      show color
    -F, --classify        append indicator (one of */=>@|) to entries

Any filter option can be combined with others. Status code is the same
with 'git status --porcelain'. In fact options -[iouadmADM] are
basically the 'git status --porcelain' with bells and whistles.
-- 
2.3.0.rc1.137.g477eb31

[PATCH 01/16] list-files: command skeleton

[Nguyễn Thái Ngọc Duy]

list-files is supposed to be the user friendly version of ls-files, or
an alternative to git-status. Nothing fancy in this patch yet.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 .gitignore                 |  1 +
 Makefile                   |  1 +
 builtin.h                  |  1 +
 builtin/list-files.c (new) | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 git.c                      |  1 +
 5 files changed, 100 insertions(+)
 create mode 100644 builtin/list-files.c

diff --git a/.gitignore b/.gitignore
index a052419..0534225 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,6 +77,7 @@
 /git-interpret-trailers
 /git-instaweb
 /git-log
+/git-list-files
 /git-ls-files
 /git-ls-remote
 /git-ls-tree
diff --git a/Makefile b/Makefile
index 459121d..17f52b2 100644
--- a/Makefile
+++ b/Makefile
@@ -832,6 +832,7 @@ BUILTIN_OBJS += builtin/index-pack.o
 BUILTIN_OBJS += builtin/init-db.o
 BUILTIN_OBJS += builtin/interpret-trailers.o
 BUILTIN_OBJS += builtin/log.o
+BUILTIN_OBJS += builtin/list-files.o
 BUILTIN_OBJS += builtin/ls-files.o
 BUILTIN_OBJS += builtin/ls-remote.o
 BUILTIN_OBJS += builtin/ls-tree.o
diff --git a/builtin.h b/builtin.h
index b87df70..afc29e7 100644
--- a/builtin.h
+++ b/builtin.h
@@ -76,6 +76,7 @@ extern int cmd_init_db(int argc, const char **argv, const char *prefix);
 extern int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);
 extern int cmd_log(int argc, const char **argv, const char *prefix);
 extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
+extern int cmd_list_files(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_remote(int argc, const char **argv, const char *prefix);
diff --git a/builtin/list-files.c b/builtin/list-files.c
new file mode 100644
index 0000000..ac33f13
--- /dev/null
+++ b/builtin/list-files.c
@@ -0,0 +1,96 @@
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "dir.h"
+
+static struct pathspec pathspec;
+static const char *prefix;
+static int prefix_length;
+
+static const char * const ls_usage[] = {
+	N_("git list-files [options] [<pathspec>...]"),
+	NULL
+};
+
+struct option ls_options[] = {
+	OPT_END()
+};
+
+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);
+	item = string_list_append(result, strbuf_detach(&sb, NULL));
+	item->util = (char *)name;
+}
+
+static void populate_cached_entries(struct string_list *result,
+				    const struct index_state *istate)
+{
+	int i;
+
+	for (i = 0; i < istate->cache_nr; i++) {
+		const struct cache_entry *ce = istate->cache[i];
+
+		if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
+				    0, NULL,
+				    S_ISDIR(ce->ce_mode) ||
+				    S_ISGITLINK(ce->ce_mode)))
+			continue;
+
+		add_one(result, ce->name);
+	}
+}
+
+static void display(const struct string_list *result)
+{
+	int i;
+
+	for (i = 0; i < result->nr; i++) {
+		const struct string_list_item *s = result->items + i;
+
+		printf("%s\n", s->string);
+	}
+}
+
+static int ls_config(const char *var, const char *value, void *cb)
+{
+	return git_default_config(var, value, cb);
+}
+
+int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
+{
+	struct string_list result = STRING_LIST_INIT_NODUP;
+
+	if (argc == 2 && !strcmp(argv[1], "-h"))
+		usage_with_options(ls_usage, ls_options);
+
+	prefix = cmd_prefix;
+	if (prefix)
+		prefix_length = strlen(prefix);
+
+	if (read_cache() < 0)
+		die(_("index file corrupt"));
+
+	git_config(ls_config, NULL);
+
+	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
+
+	parse_pathspec(&pathspec, 0,
+		       PATHSPEC_PREFER_CWD |
+		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+		       cmd_prefix, argv);
+	pathspec.max_depth = 0;
+	pathspec.recursive = 1;
+
+	refresh_index(&the_index, REFRESH_QUIET | REFRESH_UNMERGED,
+		      &pathspec, NULL, NULL);
+
+	populate_cached_entries(&result, &the_index);
+	display(&result);
+	string_list_clear(&result, 0);
+	return 0;
+}
diff --git a/git.c b/git.c
index 18fbf79..ae7fe77 100644
--- a/git.c
+++ b/git.c
@@ -418,6 +418,7 @@ static struct cmd_struct commands[] = {
 	{ "init", cmd_init_db, NO_SETUP },
 	{ "init-db", cmd_init_db, NO_SETUP },
 	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP },
+	{ "list-files", cmd_list_files, RUN_SETUP | USE_PAGER | NEED_WORK_TREE },
 	{ "log", cmd_log, RUN_SETUP },
 	{ "ls-files", cmd_ls_files, RUN_SETUP },
 	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH 01/16] list-files: command skeleton

[Junio C Hamano]

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

> list-files is supposed to be the user friendly version of ls-files, or
> an alternative to git-status. Nothing fancy in this patch yet.

The result of applying this patch alone will not give us anything
fancy, but the patch itself is interesting ;-)

> +static void populate_cached_entries(struct string_list *result,
> +				    const struct index_state *istate)
> +{
> +	int i;
> +
> +	for (i = 0; i < istate->cache_nr; i++) {
> +		const struct cache_entry *ce = istate->cache[i];
> +
> +		if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
> +				    0, NULL,
> +				    S_ISDIR(ce->ce_mode) ||
> +				    S_ISGITLINK(ce->ce_mode)))

Because we won't tell the user "You gave me Mkaefile but that did
not match" when "git list-files Mkaefile" does not produce anything,
we do not need to pass seen[] down from here.

> +	prefix = cmd_prefix;
> +	if (prefix)
> +		prefix_length = strlen(prefix);
> +
> +	if (read_cache() < 0)
> +		die(_("index file corrupt"));
> +
> +	git_config(ls_config, NULL);
> +
> +	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
> +
> +	parse_pathspec(&pathspec, 0,
> +		       PATHSPEC_PREFER_CWD |
> +		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
> +		       cmd_prefix, argv);
> +	pathspec.max_depth = 0;
> +	pathspec.recursive = 1;
> +
> +	refresh_index(&the_index, REFRESH_QUIET | REFRESH_UNMERGED,
> +		      &pathspec, NULL, NULL);

It would be better to do read-cache-preload, instead of read-cache,
if you are going to immediately refresh.  That is what "git status"
does.

> +	populate_cached_entries(&result, &the_index);
> +	display(&result);
> +	string_list_clear(&result, 0);
> +	return 0;
> +}
> diff --git a/git.c b/git.c
> index 18fbf79..ae7fe77 100644
> --- a/git.c
> +++ b/git.c
> @@ -418,6 +418,7 @@ static struct cmd_struct commands[] = {
>  	{ "init", cmd_init_db, NO_SETUP },
>  	{ "init-db", cmd_init_db, NO_SETUP },
>  	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP },
> +	{ "list-files", cmd_list_files, RUN_SETUP | USE_PAGER | NEED_WORK_TREE },

Thanks.

[PATCH 02/16] list-files: make :(glob) pathspec default

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index ac33f13..b99f2b7 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -65,6 +65,8 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 {
 	struct string_list result = STRING_LIST_INIT_NODUP;
 
+	setenv(GIT_GLOB_PATHSPECS_ENVIRONMENT, "1", 0);
+
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(ls_usage, ls_options);
 
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH 02/16] list-files: make :(glob) pathspec default

[Junio C Hamano]

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 | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/builtin/list-files.c b/builtin/list-files.c
> index ac33f13..b99f2b7 100644
> --- a/builtin/list-files.c
> +++ b/builtin/list-files.c
> @@ -65,6 +65,8 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
>  {
>  	struct string_list result = STRING_LIST_INIT_NODUP;
>  
> +	setenv(GIT_GLOB_PATHSPECS_ENVIRONMENT, "1", 0);
> +
>  	if (argc == 2 && !strcmp(argv[1], "-h"))
>  		usage_with_options(ls_usage, ls_options);

Yikes.

I do not have enough info at this step in the series to judge if it
is sensible to force the :(glob) interpretation as default, but is
it something we would want to do commonly to flip the default per
Git subcommand?  If so, using the environment feels like a clunky
way to do that.

How about a two-patch clean-up before this step?

 (1) remove the handling of literal_global and friends that peek
     into various environment variables from prefix_pathspec(),
     which is a function that is repeatedly called for each pathspec
     element given from the command line, and move that logic to
     parse_pathspec(); pass necessary information down to
     prefix_pathspec() as parameter(s);

 (2) allow parse_pathspec() so that the caller can say "the default,
     when there is no environment variable given by the end user to
     tell us otherwise, is to :(glob)".

Re: [PATCH 02/16] list-files: make :(glob) pathspec default

[Duy Nguyen]

On Fri, Mar 13, 2015 at 4:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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 | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/builtin/list-files.c b/builtin/list-files.c
>> index ac33f13..b99f2b7 100644
>> --- a/builtin/list-files.c
>> +++ b/builtin/list-files.c
>> @@ -65,6 +65,8 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
>>  {
>>       struct string_list result = STRING_LIST_INIT_NODUP;
>>
>> +     setenv(GIT_GLOB_PATHSPECS_ENVIRONMENT, "1", 0);
>> +
>>       if (argc == 2 && !strcmp(argv[1], "-h"))
>>               usage_with_options(ls_usage, ls_options);
>
> Yikes.
>
> I do not have enough info at this step in the series to judge if it
> is sensible to force the :(glob) interpretation as default, but is
> it something we would want to do commonly to flip the default per
> Git subcommand?

As this new 'ls' imitates the unix version, I try to hide this subtle
difference of git. Perhaps this is a premature move because we may
want to make :(glob) default for most commands at 3.0 or something,
except plumbing commands.

> If so, using the environment feels like a clunkyway to do that.
>
> How about a two-patch clean-up before this step?
>
>  (1) remove the handling of literal_global and friends that peek
>      into various environment variables from prefix_pathspec(),
>      which is a function that is repeatedly called for each pathspec
>      element given from the command line, and move that logic to
>      parse_pathspec(); pass necessary information down to
>      prefix_pathspec() as parameter(s);
>
>  (2) allow parse_pathspec() so that the caller can say "the default,
>      when there is no environment variable given by the end user to
>      tell us otherwise, is to :(glob)".
>

Sounds like a good thing to do anyway. Will do.
-- 
Duy

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

[Nguyễn Thái Ngọc Duy]

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;
 }
-- 
2.3.0.rc1.137.g477eb31

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

[Junio C Hamano]

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.

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

[Junio C Hamano]

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.

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

[Duy Nguyen]

On Fri, Mar 13, 2015 at 4:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?

Exactly. We would need to sort and stuff later on, so true filenames
are preserved in util->item. A cleaner way is perhaps carry all
metadata in item->util and item->string remains true filename, then do
all the formatting, coloring for all strings just before displaying.
It seems a lot varying data to carry around.

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



-- 
Duy

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

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> Exactly. We would need to sort and stuff later on, so true filenames
> are preserved in util->item. A cleaner way is perhaps carry all
> metadata in item->util and item->string remains true filename, then do
> all the formatting, coloring for all strings just before displaying.

I guess we are then in agreement with my review comment on [04/16].

Thanks.

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

[Duy Nguyen]

On Mon, Mar 16, 2015 at 4:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Exactly. We would need to sort and stuff later on, so true filenames
>> are preserved in util->item. A cleaner way is perhaps carry all
>> metadata in item->util and item->string remains true filename, then do
>> all the formatting, coloring for all strings just before displaying.
>
> I guess we are then in agreement with my review comment on [04/16].

Yes. I'm redoing it. I will probably stop using string-list as well
(it's inherited from the old code), just an array of struct.
-- 
Duy

[PATCH 04/16] list-files: add tag to each entry, filter duplicate tags

[Nguyễn Thái Ngọc Duy]

All entry strings start with two-letter tag and a space. If all
entries have the same tags, tags are not displayed.

The outcome before and after this patch is the same. But it will be
useful in future when there are more than one type of entry.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index c444a53..18af65c 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -18,12 +18,16 @@ struct option ls_options[] = {
 	OPT_END()
 };
 
-static void add_one(struct string_list *result, const char *name)
+static void add_one(struct string_list *result, const char *name,
+		    const char *tag)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list_item *item;
 
 	quote_path_relative(name, prefix_length ? prefix : NULL, &sb);
+	strbuf_insert(&sb, 0, "   ", 3);
+	sb.buf[0] = tag[0];
+	sb.buf[1] = tag[1];
 	item = string_list_append(result, strbuf_detach(&sb, NULL));
 	item->util = (char *)name;
 }
@@ -42,7 +46,38 @@ static void populate_cached_entries(struct string_list *result,
 				    S_ISGITLINK(ce->ce_mode)))
 			continue;
 
-		add_one(result, ce->name);
+		add_one(result, ce->name, "  ");
+	}
+}
+
+static void cleanup_tags(struct string_list *result)
+{
+	int i, same_1 = 1, same_2 = 1, pos, len;
+
+	for (i = 1; i < result->nr && (same_1 || same_2); i++) {
+		const char *s0 = result->items[i - 1].string;
+		const char *s1 = result->items[i].string;
+
+		same_1 = same_1 && s0[0] == s1[0];
+		same_2 = same_2 && s0[1] == s1[1];
+	}
+
+	if (same_1 && same_2) {
+		pos = 0;
+		len = 3;
+	} else if (same_1) {
+		pos = 0;
+		len = 1;
+	} else if (same_2) {
+		pos = 1;
+		len = 1;
+	} else
+		return;
+
+	for (i = 0; i < result->nr; i++) {
+		char *s = result->items[i].string;
+		int length = strlen(s);
+		memmove(s + pos, s + pos + len, length - len + 1);
 	}
 }
 
@@ -93,6 +128,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 		      &pathspec, NULL, NULL);
 
 	populate_cached_entries(&result, &the_index);
+	cleanup_tags(&result);
 	display(&result);
 	string_list_clear(&result, 0);
 	return 0;
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH 04/16] list-files: add tag to each entry, filter duplicate tags

[Junio C Hamano]

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

> All entry strings start with two-letter tag and a space. If all
> entries have the same tags, tags are not displayed.
>
> The outcome before and after this patch is the same. But it will be
> useful in future when there are more than one type of entry.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/list-files.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/list-files.c b/builtin/list-files.c
> index c444a53..18af65c 100644
> --- a/builtin/list-files.c
> +++ b/builtin/list-files.c
> @@ -18,12 +18,16 @@ struct option ls_options[] = {
>  	OPT_END()
>  };
>  
> -static void add_one(struct string_list *result, const char *name)
> +static void add_one(struct string_list *result, const char *name,
> +		    const char *tag)
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  	struct string_list_item *item;
>  
>  	quote_path_relative(name, prefix_length ? prefix : NULL, &sb);
> +	strbuf_insert(&sb, 0, "   ", 3);
> +	sb.buf[0] = tag[0];
> +	sb.buf[1] = tag[1];
>  	item = string_list_append(result, strbuf_detach(&sb, NULL));
>  	item->util = (char *)name;
>  }
> @@ -42,7 +46,38 @@ static void populate_cached_entries(struct string_list *result,
>  				    S_ISGITLINK(ce->ce_mode)))
>  			continue;
>  
> -		add_one(result, ce->name);
> +		add_one(result, ce->name, "  ");
> +	}
> +}
> +
> +static void cleanup_tags(struct string_list *result)
> +{
> +	int i, same_1 = 1, same_2 = 1, pos, len;
> +
> +	for (i = 1; i < result->nr && (same_1 || same_2); i++) {
> +		const char *s0 = result->items[i - 1].string;
> +		const char *s1 = result->items[i].string;
> +
> +		same_1 = same_1 && s0[0] == s1[0];
> +		same_2 = same_2 && s0[1] == s1[1];
> +	}
> +
> +	if (same_1 && same_2) {
> +		pos = 0;
> +		len = 3;
> +	} else if (same_1) {
> +		pos = 0;
> +		len = 1;
> +	} else if (same_2) {
> +		pos = 1;
> +		len = 1;
> +	} else
> +		return;
> +
> +	for (i = 0; i < result->nr; i++) {
> +		char *s = result->items[i].string;
> +		int length = strlen(s);
> +		memmove(s + pos, s + pos + len, length - len + 1);
>  	}
>  }

Hmm, I wonder if a different implementation strategy would produce a
code that is better for longer term maintenance and readability.
For example, how much pain would be involved if we later find that
we would want three "tag" letters per entry and wanted to add
support for that third tag by modifying this code?

Instead of half-formatted result in item->string and then inspect
and update the already formatted string at the textual level, why
not invert the keys and values of the table and arrange things this
way instead:

 * the "table" is expressed as a string-list, as this series does;

 * the keys to the table, item->string, is the original pathname;

 * the values in the table, item->util, is a pointer to a structure
   that allows implementation of list-files a more meaningful access
   to the information (as opposed to "the first column of formatted
   text output means X, the second column means Y"), perhaps like

    struct list_item {
	enum {
            LS_IS_FILE, LS_IS_DIRECTORY, LS_IS_SYMLINK,	LS_IS_SUBMODULE
	} kind;
	unsigned changed_from_index:1,
                 changed_from_HEAD:1;
	struct cache_time mtime;
    };

 * Internal processing is done to the value found in item->util, and
   the textual output is created by formatting what is in the
   *((struct list_item *)item->util) at the output phase.

A hypothetical "we need more tags" case would then involve adding a
new field (could be a bitfield "breaks_build:1") to the list_item
structure with a reasonable default, keeping the existing codepath
that does not care about the new field intact and updating only the
output phase, which would be a lot less painful, no?

> @@ -93,6 +128,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
>  		      &pathspec, NULL, NULL);
>  
>  	populate_cached_entries(&result, &the_index);
> +	cleanup_tags(&result);
>  	display(&result);
>  	string_list_clear(&result, 0);
>  	return 0;

[PATCH 05/16] list-files: add --[no-]column, -C and -1

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 18af65c..95a2e19 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -4,10 +4,12 @@
 #include "pathspec.h"
 #include "dir.h"
 #include "quote.h"
+#include "column.h"
 
 static struct pathspec pathspec;
 static const char *prefix;
 static int prefix_length;
+static unsigned int colopts;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -15,6 +17,9 @@ static const char * const ls_usage[] = {
 };
 
 struct option ls_options[] = {
+	OPT_COLUMN('C', "column", &colopts, N_("show files in columns")),
+	OPT_SET_INT('1', NULL, &colopts,
+		    N_("shortcut for --no-column"), COL_PARSEOPT),
 	OPT_END()
 };
 
@@ -85,6 +90,14 @@ static void display(const struct string_list *result)
 {
 	int i;
 
+	if (column_active(colopts)) {
+		struct column_options copts;
+		memset(&copts, 0, sizeof(copts));
+		copts.padding = 2;
+		print_columns(result, colopts, &copts);
+		return;
+	}
+
 	for (i = 0; i < result->nr; i++) {
 		const struct string_list_item *s = result->items + i;
 
@@ -94,6 +107,8 @@ static void display(const struct string_list *result)
 
 static int ls_config(const char *var, const char *value, void *cb)
 {
+	if (starts_with(var, "column."))
+		return git_column_config(var, value, "listfiles", &colopts);
 	return git_default_config(var, value, cb);
 }
 
@@ -123,6 +138,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 		       cmd_prefix, argv);
 	pathspec.max_depth = 0;
 	pathspec.recursive = 1;
+	finalize_colopts(&colopts, -1);
 
 	refresh_index(&the_index, REFRESH_QUIET | REFRESH_UNMERGED,
 		      &pathspec, NULL, NULL);
-- 
2.3.0.rc1.137.g477eb31

[PATCH 06/16] list-files: add --max-depth and -R

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 95a2e19..9a55ea6 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -10,6 +10,7 @@ static struct pathspec pathspec;
 static const char *prefix;
 static int prefix_length;
 static unsigned int colopts;
+static int max_depth;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -20,6 +21,11 @@ struct option ls_options[] = {
 	OPT_COLUMN('C', "column", &colopts, N_("show files in columns")),
 	OPT_SET_INT('1', NULL, &colopts,
 		    N_("shortcut for --no-column"), COL_PARSEOPT),
+	{ OPTION_INTEGER, 0, "max-depth", &max_depth, N_("depth"),
+	  N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
+	  NULL, 1 },
+	OPT_SET_INT('R', "recursive", &max_depth,
+		    N_("shortcut for --max-depth=-1"), -1),
 	OPT_END()
 };
 
@@ -134,9 +140,10 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	parse_pathspec(&pathspec, 0,
 		       PATHSPEC_PREFER_CWD |
+		       (max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0) |
 		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
 		       cmd_prefix, argv);
-	pathspec.max_depth = 0;
+	pathspec.max_depth = max_depth;
 	pathspec.recursive = 1;
 	finalize_colopts(&colopts, -1);
 
-- 
2.3.0.rc1.137.g477eb31

[PATCH 07/16] list-files: show directories as well as files

[Nguyễn Thái Ngọc Duy]

The index does not store directories explicitly (except submodules) so
we have to figure them out from file list when output lis depth-limited.

The function add_directory() can generate duplicate entries, which is
cleaned up before displaying.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 9a55ea6..dc865a1 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -7,10 +7,12 @@
 #include "column.h"
 
 static struct pathspec pathspec;
+static struct pathspec recursive_pathspec;
 static const char *prefix;
 static int prefix_length;
 static unsigned int colopts;
 static int max_depth;
+static int show_dirs;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -43,6 +45,51 @@ static void add_one(struct string_list *result, const char *name,
 	item->util = (char *)name;
 }
 
+static int add_directory(struct string_list *result,
+			 const char *name)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char *p;
+
+	strbuf_add(&sb, name, strlen(name));
+	while (sb.len && (p = strrchr(sb.buf, '/')) != NULL) {
+		strbuf_setlen(&sb, p - sb.buf);
+		if (!match_pathspec(&pathspec, sb.buf, sb.len, 0, NULL, 1))
+			continue;
+		add_one(result, sb.buf, "  ");
+		/*
+		 * sb.buf is leaked, but because this command is
+		 * short-lived anyway so it does not matter much
+		 */
+		return 1;
+	}
+	strbuf_release(&sb);
+	return 0;
+}
+
+static int matched(struct string_list *result, const char *name, int mode)
+{
+	int len = strlen(name);
+
+	if (!match_pathspec(&recursive_pathspec, name, len, 0, NULL,
+			    S_ISDIR(mode) || S_ISGITLINK(mode)))
+		return 0;
+
+	if (show_dirs && strchr(name, '/') &&
+	    !match_pathspec(&pathspec, name, len, 0, NULL, 1) &&
+	    add_directory(result, name))
+		return 0;
+
+	return 1;
+}
+
+static int compare_output(const void *a_, const void *b_)
+{
+	const struct string_list_item *a = a_;
+	const struct string_list_item *b = b_;
+	return strcmp(a->util, b->util);
+}
+
 static void populate_cached_entries(struct string_list *result,
 				    const struct index_state *istate)
 {
@@ -51,14 +98,16 @@ static void populate_cached_entries(struct string_list *result,
 	for (i = 0; i < istate->cache_nr; i++) {
 		const struct cache_entry *ce = istate->cache[i];
 
-		if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
-				    0, NULL,
-				    S_ISDIR(ce->ce_mode) ||
-				    S_ISGITLINK(ce->ce_mode)))
+		if (!matched(result, ce->name, ce->ce_mode))
 			continue;
 
 		add_one(result, ce->name, "  ");
 	}
+
+	if (!show_dirs)
+		return;
+	qsort(result->items, result->nr, sizeof(*result->items), compare_output);
+	string_list_remove_duplicates(result, 0);
 }
 
 static void cleanup_tags(struct string_list *result)
@@ -145,10 +194,13 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 		       cmd_prefix, argv);
 	pathspec.max_depth = max_depth;
 	pathspec.recursive = 1;
+	show_dirs = max_depth >= 0;
+	copy_pathspec(&recursive_pathspec, &pathspec);
+	recursive_pathspec.max_depth = -1;
 	finalize_colopts(&colopts, -1);
 
 	refresh_index(&the_index, REFRESH_QUIET | REFRESH_UNMERGED,
-		      &pathspec, NULL, NULL);
+		      &recursive_pathspec, NULL, NULL);
 
 	populate_cached_entries(&result, &the_index);
 	cleanup_tags(&result);
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH 07/16] list-files: show directories as well as files

[Eric Sunshine]

On Monday, March 9, 2015, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> The index does not store directories explicitly (except submodules) so
> we have to figure them out from file list when output lis depth-limited.

s/lis/is/

> The function add_directory() can generate duplicate entries, which is
> cleaned up before displaying.
> ---
> diff --git a/builtin/list-files.c b/builtin/list-files.c
> index 9a55ea6..dc865a1 100644
> --- a/builtin/list-files.c
> +++ b/builtin/list-files.c
> @@ -43,6 +45,51 @@ static void add_one(struct string_list *result, const char *name,
>         item->util = (char *)name;
>  }
>
> +static int add_directory(struct string_list *result,
> +                        const char *name)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +       const char *p;
> +
> +       strbuf_add(&sb, name, strlen(name));

strbuf_addstr() perhaps?

> +       while (sb.len && (p = strrchr(sb.buf, '/')) != NULL) {
> +               strbuf_setlen(&sb, p - sb.buf);
> +               if (!match_pathspec(&pathspec, sb.buf, sb.len, 0, NULL, 1))
> +                       continue;
> +               add_one(result, sb.buf, "  ");
> +               /*
> +                * sb.buf is leaked, but because this command is
> +                * short-lived anyway so it does not matter much
> +                */
> +               return 1;
> +       }
> +       strbuf_release(&sb);
> +       return 0;
> +}

Re: [PATCH 07/16] list-files: show directories as well as files

[Duy Nguyen]

On Tue, Mar 10, 2015 at 1:23 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> +static int add_directory(struct string_list *result,
>> +                        const char *name)
>> +{
>> +       struct strbuf sb = STRBUF_INIT;
>> +       const char *p;
>> +
>> +       strbuf_add(&sb, name, strlen(name));
>
> strbuf_addstr() perhaps?

Yeah. The previous version of this function takes string length as an
argument, but then I dropped that and used strlen() instead. Thanks
for catching.
-- 
Duy

[PATCH 08/16] list-files: add --color

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index dc865a1..316f59c 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -5,6 +5,7 @@
 #include "dir.h"
 #include "quote.h"
 #include "column.h"
+#include "color.h"
 
 static struct pathspec pathspec;
 static struct pathspec recursive_pathspec;
@@ -13,6 +14,7 @@ static int prefix_length;
 static unsigned int colopts;
 static int max_depth;
 static int show_dirs;
+static int use_color = -1;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -28,16 +30,23 @@ struct option ls_options[] = {
 	  NULL, 1 },
 	OPT_SET_INT('R', "recursive", &max_depth,
 		    N_("shortcut for --max-depth=-1"), -1),
+	OPT__COLOR(&use_color, N_("show color")),
 	OPT_END()
 };
 
-static void add_one(struct string_list *result, const char *name,
+static void add_one(struct string_list *result, const char *name, int mode,
 		    const char *tag)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list_item *item;
 
 	quote_path_relative(name, prefix_length ? prefix : NULL, &sb);
+	if (want_color(use_color)) {
+		struct strbuf quoted = STRBUF_INIT;
+		strbuf_swap(&sb, &quoted);
+		color_filename(&sb, name, quoted.buf, mode, 1);
+		strbuf_release(&quoted);
+	}
 	strbuf_insert(&sb, 0, "   ", 3);
 	sb.buf[0] = tag[0];
 	sb.buf[1] = tag[1];
@@ -56,7 +65,7 @@ static int add_directory(struct string_list *result,
 		strbuf_setlen(&sb, p - sb.buf);
 		if (!match_pathspec(&pathspec, sb.buf, sb.len, 0, NULL, 1))
 			continue;
-		add_one(result, sb.buf, "  ");
+		add_one(result, sb.buf, S_IFDIR, "  ");
 		/*
 		 * sb.buf is leaked, but because this command is
 		 * short-lived anyway so it does not matter much
@@ -101,7 +110,7 @@ static void populate_cached_entries(struct string_list *result,
 		if (!matched(result, ce->name, ce->ce_mode))
 			continue;
 
-		add_one(result, ce->name, "  ");
+		add_one(result, ce->name, ce->ce_mode, "  ");
 	}
 
 	if (!show_dirs)
@@ -164,7 +173,11 @@ static int ls_config(const char *var, const char *value, void *cb)
 {
 	if (starts_with(var, "column."))
 		return git_column_config(var, value, "listfiles", &colopts);
-	return git_default_config(var, value, cb);
+	if (!strcmp(var, "color.listfiles")) {
+		use_color = git_config_colorbool(var, value);
+		return 0;
+	}
+	return git_color_default_config(var, value, cb);
 }
 
 int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
@@ -187,6 +200,9 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
 
+	if (want_color(use_color))
+		parse_ls_color();
+
 	parse_pathspec(&pathspec, 0,
 		       PATHSPEC_PREFER_CWD |
 		       (max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0) |
-- 
2.3.0.rc1.137.g477eb31

[PATCH 09/16] list-files: add -F/--classify

[Nguyễn Thái Ngọc Duy]

This appends an indicator after the file name if it's executable, a
directory and so on, like in GNU ls. In fact append_indicator() is a
rewrite from get_type_indicator() in coreutils.git commit
7326d1f1a67edf21947ae98194f98c38b6e9e527.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 316f59c..97fa8bf 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -15,6 +15,7 @@ static unsigned int colopts;
 static int max_depth;
 static int show_dirs;
 static int use_color = -1;
+static int show_indicator;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -31,9 +32,33 @@ struct option ls_options[] = {
 	OPT_SET_INT('R', "recursive", &max_depth,
 		    N_("shortcut for --max-depth=-1"), -1),
 	OPT__COLOR(&use_color, N_("show color")),
+	OPT_BOOL('F', "classify", &show_indicator,
+		 N_("append indicator (one of */=>@|) to entries")),
 	OPT_END()
 };
 
+static void append_indicator(struct strbuf *sb, mode_t mode)
+{
+	char c = 0;
+	if (S_ISREG(mode)) {
+		if (mode & (S_IXUSR | S_IXGRP | S_IXOTH))
+			c = '*';
+	} else if (S_ISDIR(mode))
+		c = '/';
+	else if (S_ISLNK(mode))
+		c = '@';
+	else if (S_ISFIFO(mode))
+		c = '|';
+	else if (S_ISSOCK(mode))
+		c = '=';
+#ifdef S_ISDOOR
+	else if (S_ISDOOR(mode))
+		c = '>';
+#endif
+	if (c)
+		strbuf_addch(sb, c);
+}
+
 static void add_one(struct string_list *result, const char *name, int mode,
 		    const char *tag)
 {
@@ -47,6 +72,8 @@ static void add_one(struct string_list *result, const char *name, int mode,
 		color_filename(&sb, name, quoted.buf, mode, 1);
 		strbuf_release(&quoted);
 	}
+	if (show_indicator)
+		append_indicator(&sb, mode);
 	strbuf_insert(&sb, 0, "   ", 3);
 	sb.buf[0] = tag[0];
 	sb.buf[1] = tag[1];
-- 
2.3.0.rc1.137.g477eb31

[PATCH 10/16] list-files: new indicator '&' for submodules when -F is used

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 97fa8bf..eb51e7a 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -51,6 +51,8 @@ static void append_indicator(struct strbuf *sb, mode_t mode)
 		c = '|';
 	else if (S_ISSOCK(mode))
 		c = '=';
+	else if (S_ISGITLINK(mode))
+		c = '&';
 #ifdef S_ISDOOR
 	else if (S_ISDOOR(mode))
 		c = '>';
-- 
2.3.0.rc1.137.g477eb31

[PATCH 11/16] list-files: add --cached and --others

[Nguyễn Thái Ngọc Duy]

If no filter options are specified, --cached is the default.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index eb51e7a..93364b9 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -6,6 +6,7 @@
 #include "quote.h"
 #include "column.h"
 #include "color.h"
+#include "wt-status.h"
 
 static struct pathspec pathspec;
 static struct pathspec recursive_pathspec;
@@ -16,6 +17,7 @@ static int max_depth;
 static int show_dirs;
 static int use_color = -1;
 static int show_indicator;
+static int show_cached, show_untracked;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -23,6 +25,13 @@ static const char * const ls_usage[] = {
 };
 
 struct option ls_options[] = {
+	OPT_GROUP(N_("Filter options")),
+	OPT_BOOL('c', "cached", &show_cached,
+		 N_("show cached files (default)")),
+	OPT_BOOL('o', "others", &show_untracked,
+		 N_("show untracked files")),
+
+	OPT_GROUP(N_("Other")),
 	OPT_COLUMN('C', "column", &colopts, N_("show files in columns")),
 	OPT_SET_INT('1', NULL, &colopts,
 		    N_("shortcut for --no-column"), COL_PARSEOPT),
@@ -133,6 +142,9 @@ static void populate_cached_entries(struct string_list *result,
 {
 	int i;
 
+	if (!show_cached)
+		return;
+
 	for (i = 0; i < istate->cache_nr; i++) {
 		const struct cache_entry *ce = istate->cache[i];
 
@@ -148,10 +160,56 @@ static void populate_cached_entries(struct string_list *result,
 	string_list_remove_duplicates(result, 0);
 }
 
+static void populate_untracked(struct string_list *result,
+			       const struct string_list *untracked)
+{
+	int i;
+
+	for (i = 0; i < untracked->nr; i++) {
+		const struct string_list_item *item = untracked->items + i;
+		const char *name = item->string;
+		struct stat st;
+
+		if (lstat(name, &st))
+			/* color_filename() treats this as an orphan file */
+			st.st_mode = 0;
+
+		if (!matched(result, name, st.st_mode))
+			continue;
+
+		add_one(result, name, st.st_mode, "??");
+	}
+}
+
+static void wt_status_populate(struct string_list *result,
+			       struct index_state *istate)
+{
+	struct wt_status ws;
+
+	if (!show_untracked)
+		return;
+
+	wt_status_prepare(&ws);
+	copy_pathspec(&ws.pathspec, &recursive_pathspec);
+	ws.relative_paths = 0;
+	ws.use_color = 0;
+	ws.fp = NULL;
+	wt_status_collect(&ws);
+
+	if (show_untracked)
+		populate_untracked(result, &ws.untracked);
+
+	qsort(result->items, result->nr, sizeof(*result->items), compare_output);
+	string_list_remove_duplicates(result, 0);
+}
+
 static void cleanup_tags(struct string_list *result)
 {
 	int i, same_1 = 1, same_2 = 1, pos, len;
 
+	if (show_cached + show_untracked > 1)
+		return;
+
 	for (i = 1; i < result->nr && (same_1 || same_2); i++) {
 		const char *s0 = result->items[i - 1].string;
 		const char *s1 = result->items[i].string;
@@ -229,6 +287,9 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
 
+	if (!show_cached && !show_untracked)
+		show_cached = 1;
+
 	if (want_color(use_color))
 		parse_ls_color();
 
@@ -248,6 +309,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 		      &recursive_pathspec, NULL, NULL);
 
 	populate_cached_entries(&result, &the_index);
+	wt_status_populate(&result, &the_index);
 	cleanup_tags(&result);
 	display(&result);
 	string_list_clear(&result, 0);
-- 
2.3.0.rc1.137.g477eb31

[PATCH 12/16] list-files: add --ignored

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 93364b9..fbacad9 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -18,6 +18,7 @@ static int show_dirs;
 static int use_color = -1;
 static int show_indicator;
 static int show_cached, show_untracked;
+static int show_ignored;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -30,6 +31,8 @@ struct option ls_options[] = {
 		 N_("show cached files (default)")),
 	OPT_BOOL('o', "others", &show_untracked,
 		 N_("show untracked files")),
+	OPT_BOOL('i', "ignored", &show_ignored,
+		 N_("show ignored files")),
 
 	OPT_GROUP(N_("Other")),
 	OPT_COLUMN('C', "column", &colopts, N_("show files in columns")),
@@ -161,7 +164,8 @@ static void populate_cached_entries(struct string_list *result,
 }
 
 static void populate_untracked(struct string_list *result,
-			       const struct string_list *untracked)
+			       const struct string_list *untracked,
+			       const char *tag)
 {
 	int i;
 
@@ -177,7 +181,7 @@ static void populate_untracked(struct string_list *result,
 		if (!matched(result, name, st.st_mode))
 			continue;
 
-		add_one(result, name, st.st_mode, "??");
+		add_one(result, name, st.st_mode, tag);
 	}
 }
 
@@ -186,18 +190,22 @@ static void wt_status_populate(struct string_list *result,
 {
 	struct wt_status ws;
 
-	if (!show_untracked)
+	if (!show_untracked && !show_ignored)
 		return;
 
 	wt_status_prepare(&ws);
 	copy_pathspec(&ws.pathspec, &recursive_pathspec);
+	if (show_ignored)
+		ws.show_ignored_files = 1;
 	ws.relative_paths = 0;
 	ws.use_color = 0;
 	ws.fp = NULL;
 	wt_status_collect(&ws);
 
 	if (show_untracked)
-		populate_untracked(result, &ws.untracked);
+		populate_untracked(result, &ws.untracked, "??");
+	if (show_ignored)
+		populate_untracked(result, &ws.ignored, "!!");
 
 	qsort(result->items, result->nr, sizeof(*result->items), compare_output);
 	string_list_remove_duplicates(result, 0);
@@ -207,7 +215,7 @@ static void cleanup_tags(struct string_list *result)
 {
 	int i, same_1 = 1, same_2 = 1, pos, len;
 
-	if (show_cached + show_untracked > 1)
+	if (show_cached + show_untracked + show_ignored > 1)
 		return;
 
 	for (i = 1; i < result->nr && (same_1 || same_2); i++) {
@@ -287,7 +295,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
 
-	if (!show_cached && !show_untracked)
+	if (!show_cached && !show_untracked && !show_ignored)
 		show_cached = 1;
 
 	if (want_color(use_color))
-- 
2.3.0.rc1.137.g477eb31

[PATCH 13/16] list-files: add --unmerged

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index fbacad9..a82410d 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -17,7 +17,7 @@ static int max_depth;
 static int show_dirs;
 static int use_color = -1;
 static int show_indicator;
-static int show_cached, show_untracked;
+static int show_cached, show_untracked, show_unmerged;
 static int show_ignored;
 
 static const char * const ls_usage[] = {
@@ -33,6 +33,8 @@ struct option ls_options[] = {
 		 N_("show untracked files")),
 	OPT_BOOL('i', "ignored", &show_ignored,
 		 N_("show ignored files")),
+	OPT_BOOL('u', "unmerged", &show_unmerged,
+		 N_("show unmerged files")),
 
 	OPT_GROUP(N_("Other")),
 	OPT_COLUMN('C', "column", &colopts, N_("show files in columns")),
@@ -185,12 +187,46 @@ static void populate_untracked(struct string_list *result,
 	}
 }
 
+static void populate_unmerged(struct string_list *result,
+			      const struct string_list *change)
+{
+	int i;
+
+	for (i = 0; i < change->nr; i++) {
+		const struct string_list_item *item = change->items + i;
+		struct wt_status_change_data *d = item->util;
+		const char *name = item->string;
+		const char *tag;
+		struct stat st;
+
+		switch (d->stagemask) {
+		case 1: tag = "DD"; break; /* both deleted */
+		case 2: tag = "AU"; break; /* added by us */
+		case 3: tag = "UD"; break; /* deleted by them */
+		case 4: tag = "UA"; break; /* added by them */
+		case 5: tag = "DU"; break; /* deleted by us */
+		case 6: tag = "AA"; break; /* both added */
+		case 7: tag = "UU"; break; /* both modified */
+		default: continue;
+		}
+
+		if (lstat(name, &st))
+			/* color_filename() treats this as an orphan file */
+			st.st_mode = 0;
+
+		if (!matched(result, name, st.st_mode))
+			continue;
+
+		add_one(result, name, st.st_mode, tag);
+	}
+}
+
 static void wt_status_populate(struct string_list *result,
 			       struct index_state *istate)
 {
 	struct wt_status ws;
 
-	if (!show_untracked && !show_ignored)
+	if (!show_untracked && !show_ignored && !show_unmerged)
 		return;
 
 	wt_status_prepare(&ws);
@@ -206,6 +242,8 @@ static void wt_status_populate(struct string_list *result,
 		populate_untracked(result, &ws.untracked, "??");
 	if (show_ignored)
 		populate_untracked(result, &ws.ignored, "!!");
+	if (show_unmerged)
+		populate_unmerged(result, &ws.change);
 
 	qsort(result->items, result->nr, sizeof(*result->items), compare_output);
 	string_list_remove_duplicates(result, 0);
@@ -215,7 +253,8 @@ static void cleanup_tags(struct string_list *result)
 {
 	int i, same_1 = 1, same_2 = 1, pos, len;
 
-	if (show_cached + show_untracked + show_ignored > 1)
+	if (show_cached + show_untracked + show_ignored > 1 ||
+	    show_unmerged)
 		return;
 
 	for (i = 1; i < result->nr && (same_1 || same_2); i++) {
@@ -295,7 +334,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
 
-	if (!show_cached && !show_untracked && !show_ignored)
+	if (!show_cached && !show_untracked && !show_ignored && !show_unmerged)
 		show_cached = 1;
 
 	if (want_color(use_color))
-- 
2.3.0.rc1.137.g477eb31

[PATCH 14/16] list-files: add file modification options -[admADM]

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 5 deletions(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index a82410d..74836f6 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -17,8 +17,10 @@ static int max_depth;
 static int show_dirs;
 static int use_color = -1;
 static int show_indicator;
-static int show_cached, show_untracked, show_unmerged;
+static int show_cached, show_untracked, show_unmerged, show_changed;
 static int show_ignored;
+static int show_added, show_deleted, show_modified;
+static int show_wt_added, show_wt_deleted, show_wt_modified;
 
 static const char * const ls_usage[] = {
 	N_("git list-files [options] [<pathspec>...]"),
@@ -35,6 +37,18 @@ struct option ls_options[] = {
 		 N_("show ignored files")),
 	OPT_BOOL('u', "unmerged", &show_unmerged,
 		 N_("show unmerged files")),
+	OPT_BOOL('a', "added", &show_added,
+		 N_("show added files compared to HEAD")),
+	OPT_BOOL('d', "deleted", &show_deleted,
+		 N_("show deleted files compared to HEAD")),
+	OPT_BOOL('m', "modified", &show_modified,
+		 N_("show modified files compared to HEAD")),
+	OPT_BOOL('A', "wt-added", &show_wt_added,
+		 N_("show added files in worktree")),
+	OPT_BOOL('D', "wt-deleted", &show_wt_deleted,
+		 N_("show deleted files in worktree")),
+	OPT_BOOL('M', "wt-modified", &show_wt_modified,
+		 N_("show modified files on worktree")),
 
 	OPT_GROUP(N_("Other")),
 	OPT_COLUMN('C', "column", &colopts, N_("show files in columns")),
@@ -221,12 +235,52 @@ static void populate_unmerged(struct string_list *result,
 	}
 }
 
+static void populate_changed(struct string_list *result,
+			     const struct string_list *change)
+{
+	int i;
+
+	for (i = 0; i < change->nr; i++) {
+		const struct string_list_item *item = change->items + i;
+		struct wt_status_change_data *d = item->util;
+		const char *name = item->string;
+		struct stat st;
+		char tag[2];
+
+		switch (d->stagemask)
+			continue;
+
+		tag[0] = d->index_status ? d->index_status : ' ';
+		tag[1] = d->worktree_status ? d->worktree_status : ' ';
+
+		if ((show_added       && tag[0] == 'A') ||
+		    (show_deleted     && tag[0] == 'D') ||
+		    (show_modified    && tag[0] != ' ') ||
+		    (show_wt_added    && tag[1] == 'A') ||
+		    (show_wt_deleted  && tag[1] == 'D') ||
+		    (show_wt_modified && tag[1] != ' '))
+			;	/* keep going */
+		else
+			continue;
+
+		if (lstat(name, &st))
+			/* color_filename() treats this as an orphan file */
+			st.st_mode = 0;
+
+		if (!matched(result, name, st.st_mode))
+			continue;
+
+		add_one(result, name, st.st_mode, tag);
+	}
+}
+
 static void wt_status_populate(struct string_list *result,
 			       struct index_state *istate)
 {
 	struct wt_status ws;
 
-	if (!show_untracked && !show_ignored && !show_unmerged)
+	if (!show_untracked && !show_ignored &&
+	    !show_unmerged && !show_changed)
 		return;
 
 	wt_status_prepare(&ws);
@@ -244,6 +298,8 @@ static void wt_status_populate(struct string_list *result,
 		populate_untracked(result, &ws.ignored, "!!");
 	if (show_unmerged)
 		populate_unmerged(result, &ws.change);
+	if (show_changed)
+		populate_changed(result, &ws.change);
 
 	qsort(result->items, result->nr, sizeof(*result->items), compare_output);
 	string_list_remove_duplicates(result, 0);
@@ -253,8 +309,9 @@ static void cleanup_tags(struct string_list *result)
 {
 	int i, same_1 = 1, same_2 = 1, pos, len;
 
-	if (show_cached + show_untracked + show_ignored > 1 ||
-	    show_unmerged)
+	if (show_cached + show_untracked + show_ignored +
+	    show_added + show_deleted + show_wt_added + show_wt_deleted > 1 ||
+	    show_unmerged || show_modified || show_wt_modified)
 		return;
 
 	for (i = 1; i < result->nr && (same_1 || same_2); i++) {
@@ -334,7 +391,11 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);
 
-	if (!show_cached && !show_untracked && !show_ignored && !show_unmerged)
+	show_changed =
+		show_added || show_deleted || show_modified ||
+		show_wt_added || show_wt_deleted || show_wt_modified;
+	if (!show_cached && !show_untracked && !show_ignored &&
+	    !show_unmerged && !show_changed)
 		show_cached = 1;
 
 	if (want_color(use_color))
-- 
2.3.0.rc1.137.g477eb31

[PATCH 15/16] list-files: delete redundant cached entries

[Nguyễn Thái Ngọc Duy]

When both --cached and one of -amdAMD is used together we may have two
entries of the same path, e.g. "  foo" and "MM foo". In this case it's
pretty clear that "foo" must be tracked, no need to display "   foo".
The new function does that.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/builtin/list-files.c b/builtin/list-files.c
index 74836f6..49fb820 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -153,7 +153,10 @@ static int compare_output(const void *a_, const void *b_)
 {
 	const struct string_list_item *a = a_;
 	const struct string_list_item *b = b_;
-	return strcmp(a->util, b->util);
+	int ret = strcmp(a->util, b->util);
+	if (ret)
+		return ret;
+	return strncmp(a->string, b->string, 2);
 }
 
 static void populate_cached_entries(struct string_list *result,
@@ -305,6 +308,34 @@ static void wt_status_populate(struct string_list *result,
 	string_list_remove_duplicates(result, 0);
 }
 
+static void delete_duplicate_cached_entries(struct string_list *result)
+{
+	struct string_list_item *src, *dst;
+
+	if (show_unmerged || !show_cached || !show_changed)
+		return;
+
+	src = dst = result->items;
+	while (src + 1 < result->items + result->nr) {
+		const char *s0 = dst->string;
+		const char *s1 = src[1].string;
+
+		if (s0[0] == ' ' && s0[1] == ' ' &&
+		    !strcmp(s0 + 3, s1 + 3)) {
+			src++;
+		} else {
+			dst++;
+			src++;
+		}
+		if (src != dst)
+			*dst = *src;
+	}
+	if (src != dst)
+		*dst = *src;
+	result->nr = dst - result->items;
+
+}
+
 static void cleanup_tags(struct string_list *result)
 {
 	int i, same_1 = 1, same_2 = 1, pos, len;
@@ -418,6 +449,7 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix)
 
 	populate_cached_entries(&result, &the_index);
 	wt_status_populate(&result, &the_index);
+	delete_duplicate_cached_entries(&result);
 	cleanup_tags(&result);
 	display(&result);
 	string_list_clear(&result, 0);
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH 15/16] list-files: delete redundant cached entries

[Eric Sunshine]

On Monday, March 9, 2015, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> When both --cached and one of -amdAMD is used together we may have two
> entries of the same path, e.g. "  foo" and "MM foo". In this case it's
> pretty clear that "foo" must be tracked, no need to display "   foo".
> The new function does that.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/list-files.c b/builtin/list-files.c
> index 74836f6..49fb820 100644
> --- a/builtin/list-files.c
> +++ b/builtin/list-files.c
> @@ -305,6 +308,34 @@ static void wt_status_populate(struct string_list *result,
>         string_list_remove_duplicates(result, 0);
>  }
>
> +static void delete_duplicate_cached_entries(struct string_list *result)
> +{
> +       struct string_list_item *src, *dst;
> +
> +       if (show_unmerged || !show_cached || !show_changed)
> +               return;
> +
> +       src = dst = result->items;
> +       while (src + 1 < result->items + result->nr) {
> +               const char *s0 = dst->string;
> +               const char *s1 = src[1].string;
> +
> +               if (s0[0] == ' ' && s0[1] == ' ' &&
> +                   !strcmp(s0 + 3, s1 + 3)) {
> +                       src++;
> +               } else {
> +                       dst++;
> +                       src++;
> +               }
> +               if (src != dst)
> +                       *dst = *src;
> +       }
> +       if (src != dst)
> +               *dst = *src;

Do you want to take some action here (and just above) to ensure that
the item being overwritten gets deleted properly rather than leaked?

> +       result->nr = dst - result->items;
> +
> +}
> +

[PATCH 16/16] list-files: make alias 'ls' default to 'list-files'

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 config.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/config.c b/config.c
index 15a2983..16209c6 100644
--- a/config.c
+++ b/config.c
@@ -40,6 +40,10 @@ static struct config_source *cf;
 
 static int zlib_compression_seen;
 
+static const char *builtin_config =
+	"[alias]\n"
+	"	ls = list-files\n";
+
 /*
  * Default config_set that contains key-value pairs from the usual set of config
  * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG
@@ -1175,6 +1179,10 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 
 	home_config_paths(&user_config, &xdg_config, "config");
 
+	if (git_config_system())
+		git_config_from_buf(fn, "<builtin>", builtin_config,
+				    strlen(builtin_config), data);
+
 	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
 					    data);
-- 
2.3.0.rc1.137.g477eb31