threads / patch / 38762

patch, 16 partsnd/list-files redesign

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

## tl;dr

29 messages between Mar 9, 2015 and Mar 15, 2015. Diffs are folded; open one to read it.

replies: 28people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · lore
On Tue, Feb 24, 2015 at 2:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> 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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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
Show changes to 5 files +100 −0

.gitignore, Makefile, builtin.h, builtin/list-files.c, git.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
Junio C Hamano· Mar 12, 2015, 21:02 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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 ;-)

Show 12 quoted lines
> +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.

Show 20 quoted lines
> +	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.

Show 14 quoted lines
> +	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.
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to builtin/list-files.c +2 −0
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
Junio C Hamano· Mar 12, 2015, 21:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 17 quoted lines
> 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)".
Duy Nguyen· Mar 14, 2015, 11:21 UTC · re: Junio C Hamano · lore

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

On Fri, Mar 13, 2015 at 4:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 26 quoted lines
> 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.

Show 15 quoted lines
> 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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +2 −1
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
Junio C Hamano· Mar 12, 2015, 21:20 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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.

Junio C Hamano· Mar 12, 2015, 21:28 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 36 quoted lines
>>  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.
Duy Nguyen· Mar 14, 2015, 11:25 UTC · re: Junio C Hamano · lore

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

On Fri, Mar 13, 2015 at 4:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 43 quoted lines
> 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
Junio C Hamano· Mar 15, 2015, 21:16 UTC · re: Duy Nguyen · lore

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

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.
Duy Nguyen· Mar 15, 2015, 23:41 UTC · re: Junio C Hamano · lore

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

On Mon, Mar 16, 2015 at 4:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> 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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +38 −2
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
Junio C Hamano· Mar 12, 2015, 21:48 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 72 quoted lines
> 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?

Show 8 quoted lines
> @@ -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;
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
Show changes to builtin/list-files.c +16 −0
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +8 −1
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +57 −5
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
Eric Sunshine· Mar 10, 2015, 06:23 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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/
Show 18 quoted lines
> 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?
Show 14 quoted lines
> +       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;
> +}
Duy Nguyen· Mar 10, 2015, 06:39 UTC · re: Eric Sunshine · lore

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

On Tue, Mar 10, 2015 at 1:23 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 9 quoted lines
>> +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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +20 −4
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(+)
Show changes to builtin/list-files.c +27 −0
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/list-files.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to builtin/list-files.c +2 −0
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(+)
Show changes to builtin/list-files.c +62 −0
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +14 −6
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +43 −4
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +66 −5
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
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/list-files.c +33 −1
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
Eric Sunshine· Mar 10, 2015, 06:28 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

On Monday, March 9, 2015, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
Show 39 quoted lines
> 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;
> +
> +}
> +
Nguyễn Thái Ngọc Duy· Mar 9, 2015, 10:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 config.c | 8 ++++++++
 1 file changed, 8 insertions(+)
Show changes to config.c +8 −0
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

← back to recent threads