All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] parse-opt: migrate builtin-ls-files.
@ 2009-01-06  0:11 Miklos Vajna
  2009-01-06 10:22 ` Pierre Habouzit
  0 siblings, 1 reply; 19+ messages in thread
From: Miklos Vajna @ 2009-01-06  0:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

option_parse_ignored(), option_parse_directory() and
option_parse_no_empty() callbacks are necessary because
dir->show_ignored, dir->show_other_directories and
dir->hide_empty_directories are bitfields in struct dir_struct. Maybe it
would worth just removing them, but I did not want to touch dir.h just
because of parseopt.

 builtin-ls-files.c |  294 ++++++++++++++++++++++++++++------------------------
 1 files changed, 159 insertions(+), 135 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index f72eb85..a2f3de4 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -10,6 +10,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "tree.h"
+#include "parse-options.h"
 
 static int abbrev;
 static int show_deleted;
@@ -28,6 +29,7 @@ static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
+static int exc_given;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -395,156 +397,178 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_
 	return errors;
 }
 
-static const char ls_files_usage[] =
-	"git ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
-	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
-	"[ --exclude-per-directory=<filename> ] [--exclude-standard] "
-	"[--full-name] [--abbrev] [--] [<file>]*";
+static const char * const ls_files_usage[] = {
+	"git ls-files [options] [<file>]*",
+	NULL
+};
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	if (unset)
+		line_terminator = '\n';
+	else
+		line_terminator = 0;
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_exclude(arg, "", 0, &dir->exclude_list[EXC_CMDL]);
+
+	return 0;
+}
+
+static int option_parse_exclude_from(const struct option *opt,
+				     const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_excludes_from_file(dir, arg);
+
+	return 0;
+}
+
+static int option_parse_exclude_standard(const struct option *opt,
+					 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	setup_standard_excludes(dir);
+
+	return 0;
+}
+
+static int option_parse_ignored(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->show_ignored = 1;
+
+	return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+				  const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->show_other_directories = 1;
+
+	return 0;
+}
+
+static int option_parse_no_empty(const struct option *opt,
+				 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->hide_empty_directories = 1;
+
+	return 0;
+}
+
+static int option_parse_full_name(const struct option *opt,
+				  const char *arg, int unset)
+{
+	prefix_offset = 0;
+
+	return 0;
+}
 
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int exc_given = 0, require_work_tree = 0;
+	int require_work_tree = 0, show_tag = 0;
 	struct dir_struct dir;
+	struct option builtin_ls_files_options[] = {
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_BOOLEAN('t', NULL, &show_tag,
+			"identify the file status with tags"),
+		OPT_BOOLEAN('v', NULL, &show_valid_bit,
+			"use lowercase letters for 'assume unchanged' files"),
+		OPT_BOOLEAN('c', "cached", &show_cached,
+				"show cached files in the output (default)"),
+		OPT_BOOLEAN('d', "deleted", &show_deleted,
+				"show deleted files in the output"),
+		OPT_BOOLEAN('m', "modified", &show_modified,
+				"show modified files in the output"),
+		OPT_BOOLEAN('o', "others", &show_others,
+				"show other files in the output"),
+		{ OPTION_CALLBACK, 'i', "ignored", &dir, NULL,
+			"show ignored files in the output",
+			PARSE_OPT_NOARG, option_parse_ignored },
+		OPT_BOOLEAN('s', "stage", &show_stage,
+			"show staged contents' object name in the output"),
+		OPT_BOOLEAN('k', "killed", &show_killed,
+			"show files on the filesystem that need to be removed"),
+		{ OPTION_CALLBACK, 0, "directory", &dir, NULL,
+			"show 'other' directories' name only",
+			PARSE_OPT_NOARG, option_parse_directory },
+		{ OPTION_CALLBACK, 0, "no-empty-directory", &dir, NULL,
+			"do not list empty directories",
+			PARSE_OPT_NOARG, option_parse_no_empty },
+		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
+			"show unmerged files in the output"),
+		{ OPTION_CALLBACK, 'x', "exclude", &dir, "pattern",
+			"skip files matching pattern",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 'X', "exclude-from", &dir, "file",
+			"exclude patterns are read from <file>",
+			0, option_parse_exclude_from },
+		OPT_STRING(0, "exclude-per-directory", &dir.exclude_per_dir, "file",
+			"read additional per-directory exclude patterns in <file>"),
+		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
+			"add the standard git exclusions",
+			PARSE_OPT_NOARG, option_parse_exclude_standard },
+		{ OPTION_CALLBACK, 0, "full-name", NULL, NULL,
+			"make the output relative to the project top directory",
+			PARSE_OPT_NOARG, option_parse_full_name },
+		OPT_BOOLEAN(0, "error-unmatch", &error_unmatch,
+			"if any <file> is not in the index, treat this as an error"),
+		OPT_STRING(0, "with-tree", &with_tree, "tree-ish",
+			"pretend that paths removed since <tree-ish> are still present"),
+		OPT__ABBREV(&abbrev),
+		OPT_END()
+	};
 
 	memset(&dir, 0, sizeof(dir));
 	if (prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config, NULL);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_terminator = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) {
-			tag_cached = "H ";
-			tag_unmerged = "M ";
-			tag_removed = "R ";
-			tag_modified = "C ";
-			tag_other = "? ";
-			tag_killed = "K ";
-			if (arg[1] == 'v')
-				show_valid_bit = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) {
-			show_cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) {
-			show_deleted = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) {
-			show_modified = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) {
-			show_others = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
-			dir.show_ignored = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) {
-			show_stage = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
-			show_killed = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--directory")) {
-			dir.show_other_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-empty-directory")) {
-			dir.hide_empty_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
-			/* There's no point in showing unmerged unless
-			 * you also show the stage information.
-			 */
-			show_stage = 1;
-			show_unmerged = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x") && i+1 < argc) {
-			exc_given = 1;
-			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			exc_given = 1;
-			add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strcmp(arg, "-X") && i+1 < argc) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, argv[++i]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-from=")) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, arg+15);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-per-directory=")) {
-			exc_given = 1;
-			dir.exclude_per_dir = arg + 24;
-			continue;
-		}
-		if (!strcmp(arg, "--exclude-standard")) {
-			exc_given = 1;
-			setup_standard_excludes(&dir);
-			continue;
-		}
-		if (!strcmp(arg, "--full-name")) {
-			prefix_offset = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--error-unmatch")) {
-			error_unmatch = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--with-tree=")) {
-			with_tree = arg + 12;
-			continue;
-		}
-		if (!prefixcmp(arg, "--abbrev=")) {
-			abbrev = strtoul(arg+9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (abbrev > 40)
-				abbrev = 40;
-			continue;
-		}
-		if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-			continue;
-		}
-		if (*arg == '-')
-			usage(ls_files_usage);
-		break;
+	argc = parse_options(argc, argv, builtin_ls_files_options,
+			ls_files_usage, 0);
+	if (show_tag || show_valid_bit) {
+		tag_cached = "H ";
+		tag_unmerged = "M ";
+		tag_removed = "R ";
+		tag_modified = "C ";
+		tag_other = "? ";
+		tag_killed = "K ";
 	}
+	if (show_modified || show_others || dir.show_ignored || show_killed)
+		require_work_tree = 1;
+	if (show_unmerged)
+		/* There's no point in showing unmerged unless
+		 * you also show the stage information.
+		 */
+		show_stage = 1;
+	if (dir.exclude_per_dir)
+		exc_given = 1;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, argv);
 
 	/* Verify that the pathspec matches the prefix */
 	if (pathspec)
-- 
1.6.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] parse-opt: migrate builtin-ls-files.
  2009-01-06  0:11 [PATCH] parse-opt: migrate builtin-ls-files Miklos Vajna
@ 2009-01-06 10:22 ` Pierre Habouzit
  2009-01-07  3:11   ` [PATCH v2] " Miklos Vajna
  0 siblings, 1 reply; 19+ messages in thread
From: Pierre Habouzit @ 2009-01-06 10:22 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git

[-- Attachment #1: Type: text/plain, Size: 875 bytes --]

On Tue, Jan 06, 2009 at 12:11:09AM +0000, Miklos Vajna wrote:

> +static int option_parse_no_empty(const struct option *opt,
> +				 const char *arg, int unset)
> +{
> +	struct dir_struct *dir = opt->value;
> +
> +	dir->hide_empty_directories = 1;
> +
> +	return 0;
> +}

Should be option_parse_empty and deal with "unset" to know if `no-` was
prefixed to it or not.


> +		{ OPTION_CALLBACK, 0, "no-empty-directory", &dir, NULL,
> +			"do not list empty directories",

This should be "empty-directory" and "list empty directories as well"


I've not checked if you could also check more of the "unsets" things in
your callbacks as well btw, but it looks like it could.


-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] parse-opt: migrate builtin-ls-files.
  2009-01-06 10:22 ` Pierre Habouzit
@ 2009-01-07  3:11   ` Miklos Vajna
  2009-01-07 14:46     ` Pierre Habouzit
  0 siblings, 1 reply; 19+ messages in thread
From: Miklos Vajna @ 2009-01-07  3:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Tue, Jan 06, 2009 at 11:22:02AM +0100, Pierre Habouzit <madcoder@debian.org> wrote:
> > +static int option_parse_no_empty(const struct option *opt,
> > +                            const char *arg, int unset)
> > +{
> > +   struct dir_struct *dir = opt->value;
> > +
> > +   dir->hide_empty_directories = 1;
> > +
> > +   return 0;
> > +}
>
> Should be option_parse_empty and deal with "unset" to know if `no-`
> was
> prefixed to it or not.
>
>
> > +           { OPTION_CALLBACK, 0, "no-empty-directory", &dir, NULL,
> > +                   "do not list empty directories",
>
> This should be "empty-directory" and "list empty directories as well"

Ah, sure.

> I've not checked if you could also check more of the "unsets" things
> in
> your callbacks as well btw, but it looks like it could.

Right, added to option_parse_ignored() and option_parse_directory() as
well.

Interdiff: b3b6ad0..a44941c in git://repo.or.cz/git/vmiklos.git.

 builtin-ls-files.c |  303 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 168 insertions(+), 135 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index f72eb85..8a946ef 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -10,6 +10,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "tree.h"
+#include "parse-options.h"
 
 static int abbrev;
 static int show_deleted;
@@ -28,6 +29,7 @@ static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
+static int exc_given;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -395,156 +397,187 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_
 	return errors;
 }
 
-static const char ls_files_usage[] =
-	"git ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
-	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
-	"[ --exclude-per-directory=<filename> ] [--exclude-standard] "
-	"[--full-name] [--abbrev] [--] [<file>]*";
+static const char * const ls_files_usage[] = {
+	"git ls-files [options] [<file>]*",
+	NULL
+};
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	if (unset)
+		line_terminator = '\n';
+	else
+		line_terminator = 0;
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_exclude(arg, "", 0, &dir->exclude_list[EXC_CMDL]);
+
+	return 0;
+}
+
+static int option_parse_exclude_from(const struct option *opt,
+				     const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_excludes_from_file(dir, arg);
+
+	return 0;
+}
+
+static int option_parse_exclude_standard(const struct option *opt,
+					 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	setup_standard_excludes(dir);
+
+	return 0;
+}
+
+static int option_parse_ignored(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	if (unset)
+		dir->show_ignored = 0;
+	else
+		dir->show_ignored = 1;
+
+	return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+				  const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	if (unset)
+		dir->show_other_directories = 0;
+	else
+		dir->show_other_directories = 1;
+
+	return 0;
+}
+
+static int option_parse_empty(const struct option *opt,
+				 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	if (unset)
+		dir->hide_empty_directories = 1;
+	else
+		dir->hide_empty_directories = 0;
+
+	return 0;
+}
+
+static int option_parse_full_name(const struct option *opt,
+				  const char *arg, int unset)
+{
+	prefix_offset = 0;
+
+	return 0;
+}
 
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int exc_given = 0, require_work_tree = 0;
+	int require_work_tree = 0, show_tag = 0;
 	struct dir_struct dir;
+	struct option builtin_ls_files_options[] = {
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_BOOLEAN('t', NULL, &show_tag,
+			"identify the file status with tags"),
+		OPT_BOOLEAN('v', NULL, &show_valid_bit,
+			"use lowercase letters for 'assume unchanged' files"),
+		OPT_BOOLEAN('c', "cached", &show_cached,
+				"show cached files in the output (default)"),
+		OPT_BOOLEAN('d', "deleted", &show_deleted,
+				"show deleted files in the output"),
+		OPT_BOOLEAN('m', "modified", &show_modified,
+				"show modified files in the output"),
+		OPT_BOOLEAN('o', "others", &show_others,
+				"show other files in the output"),
+		{ OPTION_CALLBACK, 'i', "ignored", &dir, NULL,
+			"show ignored files in the output",
+			PARSE_OPT_NOARG, option_parse_ignored },
+		OPT_BOOLEAN('s', "stage", &show_stage,
+			"show staged contents' object name in the output"),
+		OPT_BOOLEAN('k', "killed", &show_killed,
+			"show files on the filesystem that need to be removed"),
+		{ OPTION_CALLBACK, 0, "directory", &dir, NULL,
+			"show 'other' directories' name only",
+			PARSE_OPT_NOARG, option_parse_directory },
+		{ OPTION_CALLBACK, 0, "empty-directory", &dir, NULL,
+			"list empty directories",
+			PARSE_OPT_NOARG, option_parse_empty },
+		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
+			"show unmerged files in the output"),
+		{ OPTION_CALLBACK, 'x', "exclude", &dir, "pattern",
+			"skip files matching pattern",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 'X', "exclude-from", &dir, "file",
+			"exclude patterns are read from <file>",
+			0, option_parse_exclude_from },
+		OPT_STRING(0, "exclude-per-directory", &dir.exclude_per_dir, "file",
+			"read additional per-directory exclude patterns in <file>"),
+		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
+			"add the standard git exclusions",
+			PARSE_OPT_NOARG, option_parse_exclude_standard },
+		{ OPTION_CALLBACK, 0, "full-name", NULL, NULL,
+			"make the output relative to the project top directory",
+			PARSE_OPT_NOARG, option_parse_full_name },
+		OPT_BOOLEAN(0, "error-unmatch", &error_unmatch,
+			"if any <file> is not in the index, treat this as an error"),
+		OPT_STRING(0, "with-tree", &with_tree, "tree-ish",
+			"pretend that paths removed since <tree-ish> are still present"),
+		OPT__ABBREV(&abbrev),
+		OPT_END()
+	};
 
 	memset(&dir, 0, sizeof(dir));
 	if (prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config, NULL);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_terminator = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) {
-			tag_cached = "H ";
-			tag_unmerged = "M ";
-			tag_removed = "R ";
-			tag_modified = "C ";
-			tag_other = "? ";
-			tag_killed = "K ";
-			if (arg[1] == 'v')
-				show_valid_bit = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) {
-			show_cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) {
-			show_deleted = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) {
-			show_modified = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) {
-			show_others = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
-			dir.show_ignored = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) {
-			show_stage = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
-			show_killed = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--directory")) {
-			dir.show_other_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-empty-directory")) {
-			dir.hide_empty_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
-			/* There's no point in showing unmerged unless
-			 * you also show the stage information.
-			 */
-			show_stage = 1;
-			show_unmerged = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x") && i+1 < argc) {
-			exc_given = 1;
-			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			exc_given = 1;
-			add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strcmp(arg, "-X") && i+1 < argc) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, argv[++i]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-from=")) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, arg+15);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-per-directory=")) {
-			exc_given = 1;
-			dir.exclude_per_dir = arg + 24;
-			continue;
-		}
-		if (!strcmp(arg, "--exclude-standard")) {
-			exc_given = 1;
-			setup_standard_excludes(&dir);
-			continue;
-		}
-		if (!strcmp(arg, "--full-name")) {
-			prefix_offset = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--error-unmatch")) {
-			error_unmatch = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--with-tree=")) {
-			with_tree = arg + 12;
-			continue;
-		}
-		if (!prefixcmp(arg, "--abbrev=")) {
-			abbrev = strtoul(arg+9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (abbrev > 40)
-				abbrev = 40;
-			continue;
-		}
-		if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-			continue;
-		}
-		if (*arg == '-')
-			usage(ls_files_usage);
-		break;
+	argc = parse_options(argc, argv, builtin_ls_files_options,
+			ls_files_usage, 0);
+	if (show_tag || show_valid_bit) {
+		tag_cached = "H ";
+		tag_unmerged = "M ";
+		tag_removed = "R ";
+		tag_modified = "C ";
+		tag_other = "? ";
+		tag_killed = "K ";
 	}
+	if (show_modified || show_others || dir.show_ignored || show_killed)
+		require_work_tree = 1;
+	if (show_unmerged)
+		/* There's no point in showing unmerged unless
+		 * you also show the stage information.
+		 */
+		show_stage = 1;
+	if (dir.exclude_per_dir)
+		exc_given = 1;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, argv);
 
 	/* Verify that the pathspec matches the prefix */
 	if (pathspec)
-- 
1.6.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] parse-opt: migrate builtin-ls-files.
  2009-01-07  3:11   ` [PATCH v2] " Miklos Vajna
@ 2009-01-07 14:46     ` Pierre Habouzit
  2009-01-08  0:55       ` [PATCH v3] " Miklos Vajna
  0 siblings, 1 reply; 19+ messages in thread
From: Pierre Habouzit @ 2009-01-07 14:46 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On Wed, Jan 07, 2009 at 03:11:34AM +0000, Miklos Vajna wrote:

> +static int option_parse_ignored(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	struct dir_struct *dir = opt->value;
> +
> +	if (unset)
> +		dir->show_ignored = 0;
> +	else
> +		dir->show_ignored = 1;

dir->show_ignored = !unset ?

> +static int option_parse_directory(const struct option *opt,
> +				  const char *arg, int unset)
> +{

ditto

> +static int option_parse_empty(const struct option *opt,
> +				 const char *arg, int unset)
> +{

ditto


-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-07 14:46     ` Pierre Habouzit
@ 2009-01-08  0:55       ` Miklos Vajna
  2009-01-08  1:10         ` Johannes Schindelin
                           ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Miklos Vajna @ 2009-01-08  0:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Wed, Jan 07, 2009 at 03:46:40PM +0100, Pierre Habouzit <madcoder@debian.org> wrote:
> > +   if (unset)
> > +           dir->show_ignored = 0;
> > +   else
> > +           dir->show_ignored = 1;
>
> dir->show_ignored = !unset ?

True, cleaned up all 3 occurrences.

Interdiff: git diff b2a38d9..ee34fcc in my repo.

 builtin-ls-files.c |  294 ++++++++++++++++++++++++++++------------------------
 1 files changed, 159 insertions(+), 135 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index f72eb85..ffa8210 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -10,6 +10,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "tree.h"
+#include "parse-options.h"
 
 static int abbrev;
 static int show_deleted;
@@ -28,6 +29,7 @@ static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
+static int exc_given;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -395,156 +397,178 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_
 	return errors;
 }
 
-static const char ls_files_usage[] =
-	"git ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
-	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
-	"[ --exclude-per-directory=<filename> ] [--exclude-standard] "
-	"[--full-name] [--abbrev] [--] [<file>]*";
+static const char * const ls_files_usage[] = {
+	"git ls-files [options] [<file>]*",
+	NULL
+};
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	if (unset)
+		line_terminator = '\n';
+	else
+		line_terminator = 0;
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_exclude(arg, "", 0, &dir->exclude_list[EXC_CMDL]);
+
+	return 0;
+}
+
+static int option_parse_exclude_from(const struct option *opt,
+				     const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_excludes_from_file(dir, arg);
+
+	return 0;
+}
+
+static int option_parse_exclude_standard(const struct option *opt,
+					 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	setup_standard_excludes(dir);
+
+	return 0;
+}
+
+static int option_parse_ignored(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->show_ignored = !unset;
+
+	return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+				  const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->show_other_directories = !unset;
+
+	return 0;
+}
+
+static int option_parse_empty(const struct option *opt,
+				 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->hide_empty_directories = unset;
+
+	return 0;
+}
+
+static int option_parse_full_name(const struct option *opt,
+				  const char *arg, int unset)
+{
+	prefix_offset = 0;
+
+	return 0;
+}
 
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int exc_given = 0, require_work_tree = 0;
+	int require_work_tree = 0, show_tag = 0;
 	struct dir_struct dir;
+	struct option builtin_ls_files_options[] = {
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_BOOLEAN('t', NULL, &show_tag,
+			"identify the file status with tags"),
+		OPT_BOOLEAN('v', NULL, &show_valid_bit,
+			"use lowercase letters for 'assume unchanged' files"),
+		OPT_BOOLEAN('c', "cached", &show_cached,
+				"show cached files in the output (default)"),
+		OPT_BOOLEAN('d', "deleted", &show_deleted,
+				"show deleted files in the output"),
+		OPT_BOOLEAN('m', "modified", &show_modified,
+				"show modified files in the output"),
+		OPT_BOOLEAN('o', "others", &show_others,
+				"show other files in the output"),
+		{ OPTION_CALLBACK, 'i', "ignored", &dir, NULL,
+			"show ignored files in the output",
+			PARSE_OPT_NOARG, option_parse_ignored },
+		OPT_BOOLEAN('s', "stage", &show_stage,
+			"show staged contents' object name in the output"),
+		OPT_BOOLEAN('k', "killed", &show_killed,
+			"show files on the filesystem that need to be removed"),
+		{ OPTION_CALLBACK, 0, "directory", &dir, NULL,
+			"show 'other' directories' name only",
+			PARSE_OPT_NOARG, option_parse_directory },
+		{ OPTION_CALLBACK, 0, "empty-directory", &dir, NULL,
+			"list empty directories",
+			PARSE_OPT_NOARG, option_parse_empty },
+		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
+			"show unmerged files in the output"),
+		{ OPTION_CALLBACK, 'x', "exclude", &dir, "pattern",
+			"skip files matching pattern",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 'X', "exclude-from", &dir, "file",
+			"exclude patterns are read from <file>",
+			0, option_parse_exclude_from },
+		OPT_STRING(0, "exclude-per-directory", &dir.exclude_per_dir, "file",
+			"read additional per-directory exclude patterns in <file>"),
+		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
+			"add the standard git exclusions",
+			PARSE_OPT_NOARG, option_parse_exclude_standard },
+		{ OPTION_CALLBACK, 0, "full-name", NULL, NULL,
+			"make the output relative to the project top directory",
+			PARSE_OPT_NOARG, option_parse_full_name },
+		OPT_BOOLEAN(0, "error-unmatch", &error_unmatch,
+			"if any <file> is not in the index, treat this as an error"),
+		OPT_STRING(0, "with-tree", &with_tree, "tree-ish",
+			"pretend that paths removed since <tree-ish> are still present"),
+		OPT__ABBREV(&abbrev),
+		OPT_END()
+	};
 
 	memset(&dir, 0, sizeof(dir));
 	if (prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config, NULL);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_terminator = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) {
-			tag_cached = "H ";
-			tag_unmerged = "M ";
-			tag_removed = "R ";
-			tag_modified = "C ";
-			tag_other = "? ";
-			tag_killed = "K ";
-			if (arg[1] == 'v')
-				show_valid_bit = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) {
-			show_cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) {
-			show_deleted = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) {
-			show_modified = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) {
-			show_others = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
-			dir.show_ignored = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) {
-			show_stage = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
-			show_killed = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--directory")) {
-			dir.show_other_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-empty-directory")) {
-			dir.hide_empty_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
-			/* There's no point in showing unmerged unless
-			 * you also show the stage information.
-			 */
-			show_stage = 1;
-			show_unmerged = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x") && i+1 < argc) {
-			exc_given = 1;
-			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			exc_given = 1;
-			add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strcmp(arg, "-X") && i+1 < argc) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, argv[++i]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-from=")) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, arg+15);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-per-directory=")) {
-			exc_given = 1;
-			dir.exclude_per_dir = arg + 24;
-			continue;
-		}
-		if (!strcmp(arg, "--exclude-standard")) {
-			exc_given = 1;
-			setup_standard_excludes(&dir);
-			continue;
-		}
-		if (!strcmp(arg, "--full-name")) {
-			prefix_offset = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--error-unmatch")) {
-			error_unmatch = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--with-tree=")) {
-			with_tree = arg + 12;
-			continue;
-		}
-		if (!prefixcmp(arg, "--abbrev=")) {
-			abbrev = strtoul(arg+9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (abbrev > 40)
-				abbrev = 40;
-			continue;
-		}
-		if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-			continue;
-		}
-		if (*arg == '-')
-			usage(ls_files_usage);
-		break;
+	argc = parse_options(argc, argv, builtin_ls_files_options,
+			ls_files_usage, 0);
+	if (show_tag || show_valid_bit) {
+		tag_cached = "H ";
+		tag_unmerged = "M ";
+		tag_removed = "R ";
+		tag_modified = "C ";
+		tag_other = "? ";
+		tag_killed = "K ";
 	}
+	if (show_modified || show_others || dir.show_ignored || show_killed)
+		require_work_tree = 1;
+	if (show_unmerged)
+		/* There's no point in showing unmerged unless
+		 * you also show the stage information.
+		 */
+		show_stage = 1;
+	if (dir.exclude_per_dir)
+		exc_given = 1;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, argv);
 
 	/* Verify that the pathspec matches the prefix */
 	if (pathspec)
-- 
1.6.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-08  0:55       ` [PATCH v3] " Miklos Vajna
@ 2009-01-08  1:10         ` Johannes Schindelin
  2009-01-08  1:54           ` Miklos Vajna
  2009-01-15  0:14         ` Miklos Vajna
  2009-02-14 20:56         ` Johannes Schindelin
  2 siblings, 1 reply; 19+ messages in thread
From: Johannes Schindelin @ 2009-01-08  1:10 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, Pierre Habouzit, git

Hi,

On Thu, 8 Jan 2009, Miklos Vajna wrote:

> Interdiff: git diff b2a38d9..ee34fcc in my repo.

... or on repo.or.cz:

http://repo.or.cz/w/git/vmiklos.git?a=treediff;hpb=b2a38d9;hb=ee34fcc

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-08  1:10         ` Johannes Schindelin
@ 2009-01-08  1:54           ` Miklos Vajna
  0 siblings, 0 replies; 19+ messages in thread
From: Miklos Vajna @ 2009-01-08  1:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Pierre Habouzit, git

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

On Thu, Jan 08, 2009 at 02:10:15AM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Interdiff: git diff b2a38d9..ee34fcc in my repo.
> 
> ... or on repo.or.cz:
> 
> http://repo.or.cz/w/git/vmiklos.git?a=treediff;hpb=b2a38d9;hb=ee34fcc

That *is* what I meant by 'my repo'. ;-)

BTW thanks for the link, I didn't know about it - AFAIK stock gitweb
does not have a treediff view.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-08  0:55       ` [PATCH v3] " Miklos Vajna
  2009-01-08  1:10         ` Johannes Schindelin
@ 2009-01-15  0:14         ` Miklos Vajna
  2009-01-15  3:16           ` Junio C Hamano
  2009-02-14 20:56         ` Johannes Schindelin
  2 siblings, 1 reply; 19+ messages in thread
From: Miklos Vajna @ 2009-01-15  0:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

On Thu, Jan 08, 2009 at 01:55:45AM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
>  builtin-ls-files.c |  294 ++++++++++++++++++++++++++++------------------------
>  1 files changed, 159 insertions(+), 135 deletions(-)

Hi Junio,

Was this dropped on the floor by accident?

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-15  0:14         ` Miklos Vajna
@ 2009-01-15  3:16           ` Junio C Hamano
  2009-01-15  3:48             ` Miklos Vajna
  2009-02-14 12:16             ` Miklos Vajna
  0 siblings, 2 replies; 19+ messages in thread
From: Junio C Hamano @ 2009-01-15  3:16 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Pierre Habouzit, git

Miklos Vajna <vmiklos@frugalware.org> writes:

> Was this dropped on the floor by accident?

I am not fundamentally opposed to the parseopt conversion, but I was
somewhat discouraged from taking another one, after we got burned by the
one that converted git-apply without much visible gain but with a new bug.

Because ls-files is a plumbing, it has somewhat lower priority for user
friendliness than any other patches currently in-flight on the list; hence
it has been backburnered.  It still is kept in my Inbox.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-15  3:16           ` Junio C Hamano
@ 2009-01-15  3:48             ` Miklos Vajna
  2009-02-14 12:16             ` Miklos Vajna
  1 sibling, 0 replies; 19+ messages in thread
From: Miklos Vajna @ 2009-01-15  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On Wed, Jan 14, 2009 at 07:16:38PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I am not fundamentally opposed to the parseopt conversion, but I was
> somewhat discouraged from taking another one, after we got burned by the
> one that converted git-apply without much visible gain but with a new bug.
> 
> Because ls-files is a plumbing, it has somewhat lower priority for user
> friendliness than any other patches currently in-flight on the list; hence
> it has been backburnered.  It still is kept in my Inbox.

Makes sense, thanks for the answer.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-15  3:16           ` Junio C Hamano
  2009-01-15  3:48             ` Miklos Vajna
@ 2009-02-14 12:16             ` Miklos Vajna
  2009-02-14 20:04               ` Junio C Hamano
  1 sibling, 1 reply; 19+ messages in thread
From: Miklos Vajna @ 2009-02-14 12:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

On Wed, Jan 14, 2009 at 07:16:38PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
> 
> > Was this dropped on the floor by accident?
> 
> I am not fundamentally opposed to the parseopt conversion, but I was
> somewhat discouraged from taking another one, after we got burned by the
> one that converted git-apply without much visible gain but with a new bug.
> 
> Because ls-files is a plumbing, it has somewhat lower priority for user
> friendliness than any other patches currently in-flight on the list; hence
> it has been backburnered.  It still is kept in my Inbox.

I'm just asking again as I see the parseopt patch for builtin-config now
on the list.

Should I just resend this patch after v1.6.2?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-02-14 12:16             ` Miklos Vajna
@ 2009-02-14 20:04               ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2009-02-14 20:04 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Pierre Habouzit, git

Miklos Vajna <vmiklos@frugalware.org> writes:

> On Wed, Jan 14, 2009 at 07:16:38PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
>> Miklos Vajna <vmiklos@frugalware.org> writes:
>> 
>> > Was this dropped on the floor by accident?
>> 
>> I am not fundamentally opposed to the parseopt conversion, but I was
>> somewhat discouraged from taking another one, after we got burned by the
>> one that converted git-apply without much visible gain but with a new bug.
>> 
>> Because ls-files is a plumbing, it has somewhat lower priority for user
>> friendliness than any other patches currently in-flight on the list; hence
>> it has been backburnered.  It still is kept in my Inbox.
>
> I'm just asking again as I see the parseopt patch for builtin-config now
> on the list.
>
> Should I just resend this patch after v1.6.2?

You can do it either way, and a resend after v1.6.2 is certainly
appreciated,

As I said earlier in "What's cooking", 'pu' and 'next' are open during
this freeze cycle as an experiment.

I limit my bandwidth for handing anything non-fix during the rc freeze
period as always.  The only difference from previous cycles is that the
cap used to be set to near-absolute-zero, but in this cycle it is not.

It still is capped and updates to 'pu' and 'next' are "as time permits"
basis.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
  2009-01-08  0:55       ` [PATCH v3] " Miklos Vajna
  2009-01-08  1:10         ` Johannes Schindelin
  2009-01-15  0:14         ` Miklos Vajna
@ 2009-02-14 20:56         ` Johannes Schindelin
  2009-02-15 19:54           ` [PATCH] " Miklos Vajna
  2 siblings, 1 reply; 19+ messages in thread
From: Johannes Schindelin @ 2009-02-14 20:56 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, Pierre Habouzit, git

Hi,

On Thu, 8 Jan 2009, Miklos Vajna wrote:

> +static int option_parse_z(const struct option *opt,
> +			  const char *arg, int unset)
> +{
> +	if (unset)
> +		line_terminator = '\n';
> +	else
> +		line_terminator = 0;
> +	return 0;
> +}

	line_terminator = unset ? '\0' : '\n';

Hmm?

> +static int option_parse_exclude(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	struct dir_struct *dir = opt->value;
> +
> +	exc_given = 1;
> +	add_exclude(arg, "", 0, &dir->exclude_list[EXC_CMDL]);

Why is &dir->exclude_list[EXC_CMDL] not stored in opt->value directly?

> +static int option_parse_ignored(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	struct dir_struct *dir = opt->value;
> +
> +	dir->show_ignored = !unset;
> +
> +	return 0;
> +}

Maybe this wants to be converted into an OPTION_BIT compatible data type?

> +static int option_parse_directory(const struct option *opt,
> +				  const char *arg, int unset)
> +{
> +	struct dir_struct *dir = opt->value;
> +
> +	dir->show_other_directories = !unset;
> +
> +	return 0;
> +}

Likewise?

> +static int option_parse_empty(const struct option *opt,
> +				 const char *arg, int unset)
> +{
> +	struct dir_struct *dir = opt->value;
> +
> +	dir->hide_empty_directories = unset;
> +
> +	return 0;
> +}

Maybe we need an OPT_BIT_NEG?

> +static int option_parse_full_name(const struct option *opt,
> +				  const char *arg, int unset)
> +{
> +	prefix_offset = 0;
> +
> +	return 0;
> +}

Not that it matters much, but maybe you can guard this with an
"if (!unset)"?

> +		{ OPTION_CALLBACK, 0, "full-name", NULL, NULL,
> +			"make the output relative to the project top directory",
> +			PARSE_OPT_NOARG, option_parse_full_name },

Maybe OPT_NONEG, and maybe SET_INT?

> +	if (dir.exclude_per_dir)
> +		exc_given = 1;

You could use a boolean to handle --exclude-standard, too... But you did 
not do that so that there is no regression with specific ordering of the 
exclude options, right?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] parse-opt: migrate builtin-ls-files.
  2009-02-14 20:56         ` Johannes Schindelin
@ 2009-02-15 19:54           ` Miklos Vajna
  2009-02-15 20:13             ` Johannes Schindelin
  0 siblings, 1 reply; 19+ messages in thread
From: Miklos Vajna @ 2009-02-15 19:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Pierre Habouzit, git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Sat, Feb 14, 2009 at 09:56:04PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 8 Jan 2009, Miklos Vajna wrote:
>
> > +static int option_parse_z(const struct option *opt,
> > +                     const char *arg, int unset)
> > +{
> > +   if (unset)
> > +           line_terminator = '\n';
> > +   else
> > +           line_terminator = 0;
> > +   return 0;
> > +}
>
>       line_terminator = unset ? '\0' : '\n';
>
> Hmm?

The opposite:

        line_terminator = unset ? '\n' : '\0';

> > +static int option_parse_exclude(const struct option *opt,
> > +                           const char *arg, int unset)
> > +{
> > +   struct dir_struct *dir = opt->value;
> > +
> > +   exc_given = 1;
> > +   add_exclude(arg, "", 0, &dir->exclude_list[EXC_CMDL]);
>
> Why is &dir->exclude_list[EXC_CMDL] not stored in opt->value directly?

Changed.

> > +static int option_parse_ignored(const struct option *opt,
> > +                           const char *arg, int unset)
> > +{
> > +   struct dir_struct *dir = opt->value;
> > +
> > +   dir->show_ignored = !unset;
> > +
> > +   return 0;
> > +}
>
> Maybe this wants to be converted into an OPTION_BIT compatible data
> type?

I think that's not possible, as show_ignored is a bitfield.

> > +static int option_parse_directory(const struct option *opt,
> > +                             const char *arg, int unset)
> > +{
> > +   struct dir_struct *dir = opt->value;
> > +
> > +   dir->show_other_directories = !unset;
> > +
> > +   return 0;
> > +}
>
> Likewise?

Same, show_other_directories can't be passed as a pointer, either.

> > +static int option_parse_empty(const struct option *opt,
> > +                            const char *arg, int unset)
> > +{
> > +   struct dir_struct *dir = opt->value;
> > +
> > +   dir->hide_empty_directories = unset;
> > +
> > +   return 0;
> > +}
>
> Maybe we need an OPT_BIT_NEG?

I can do it, but hide_empty_directories is a bitfield as well.

> > +           { OPTION_CALLBACK, 0, "full-name", NULL, NULL,
> > +                   "make the output relative to the project top
> > directory",
> > +                   PARSE_OPT_NOARG, option_parse_full_name },
>
> Maybe OPT_NONEG, and maybe SET_INT?

Ah yes, that makes option_parse_full_name useless, great. :-)

> > +   if (dir.exclude_per_dir)
> > +           exc_given = 1;
>
> You could use a boolean to handle --exclude-standard, too... But you
> did
> not do that so that there is no regression with specific ordering of
> the
> exclude options, right?

Yes, exactly.

 builtin-ls-files.c |  284 +++++++++++++++++++++++++++-------------------------
 1 files changed, 149 insertions(+), 135 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 9dec282..0070669 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -10,6 +10,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "tree.h"
+#include "parse-options.h"
 
 static int abbrev;
 static int show_deleted;
@@ -28,6 +29,7 @@ static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
+static int exc_given;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -374,156 +376,168 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_
 	return errors;
 }
 
-static const char ls_files_usage[] =
-	"git ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
-	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
-	"[ --exclude-per-directory=<filename> ] [--exclude-standard] "
-	"[--full-name] [--abbrev] [--] [<file>]*";
+static const char * const ls_files_usage[] = {
+	"git ls-files [options] [<file>]*",
+	NULL
+};
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	line_terminator = unset ? '\n' : '\0';
+
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct exclude_list *list = opt->value;
+
+	exc_given = 1;
+	add_exclude(arg, "", 0, list);
+
+	return 0;
+}
+
+static int option_parse_exclude_from(const struct option *opt,
+				     const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_excludes_from_file(dir, arg);
+
+	return 0;
+}
+
+static int option_parse_exclude_standard(const struct option *opt,
+					 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	setup_standard_excludes(dir);
+
+	return 0;
+}
+
+static int option_parse_ignored(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->show_ignored = !unset;
+
+	return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+				  const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->show_other_directories = !unset;
+
+	return 0;
+}
+
+static int option_parse_empty(const struct option *opt,
+				 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	dir->hide_empty_directories = unset;
+
+	return 0;
+}
 
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int exc_given = 0, require_work_tree = 0;
+	int require_work_tree = 0, show_tag = 0;
 	struct dir_struct dir;
+	struct option builtin_ls_files_options[] = {
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_BOOLEAN('t', NULL, &show_tag,
+			"identify the file status with tags"),
+		OPT_BOOLEAN('v', NULL, &show_valid_bit,
+			"use lowercase letters for 'assume unchanged' files"),
+		OPT_BOOLEAN('c', "cached", &show_cached,
+				"show cached files in the output (default)"),
+		OPT_BOOLEAN('d', "deleted", &show_deleted,
+				"show deleted files in the output"),
+		OPT_BOOLEAN('m', "modified", &show_modified,
+				"show modified files in the output"),
+		OPT_BOOLEAN('o', "others", &show_others,
+				"show other files in the output"),
+		{ OPTION_CALLBACK, 'i', "ignored", &dir, NULL,
+			"show ignored files in the output",
+			PARSE_OPT_NOARG, option_parse_ignored },
+		OPT_BOOLEAN('s', "stage", &show_stage,
+			"show staged contents' object name in the output"),
+		OPT_BOOLEAN('k', "killed", &show_killed,
+			"show files on the filesystem that need to be removed"),
+		{ OPTION_CALLBACK, 0, "directory", &dir, NULL,
+			"show 'other' directories' name only",
+			PARSE_OPT_NOARG, option_parse_directory },
+		{ OPTION_CALLBACK, 0, "empty-directory", &dir, NULL,
+			"list empty directories",
+			PARSE_OPT_NOARG, option_parse_empty },
+		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
+			"show unmerged files in the output"),
+		{ OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], "pattern",
+			"skip files matching pattern",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 'X', "exclude-from", &dir, "file",
+			"exclude patterns are read from <file>",
+			0, option_parse_exclude_from },
+		OPT_STRING(0, "exclude-per-directory", &dir.exclude_per_dir, "file",
+			"read additional per-directory exclude patterns in <file>"),
+		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
+			"add the standard git exclusions",
+			PARSE_OPT_NOARG, option_parse_exclude_standard },
+		{ OPTION_SET_INT, 0, "full-name", &prefix_offset, NULL,
+			"make the output relative to the project top directory",
+			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+		OPT_BOOLEAN(0, "error-unmatch", &error_unmatch,
+			"if any <file> is not in the index, treat this as an error"),
+		OPT_STRING(0, "with-tree", &with_tree, "tree-ish",
+			"pretend that paths removed since <tree-ish> are still present"),
+		OPT__ABBREV(&abbrev),
+		OPT_END()
+	};
 
 	memset(&dir, 0, sizeof(dir));
 	if (prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config, NULL);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_terminator = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) {
-			tag_cached = "H ";
-			tag_unmerged = "M ";
-			tag_removed = "R ";
-			tag_modified = "C ";
-			tag_other = "? ";
-			tag_killed = "K ";
-			if (arg[1] == 'v')
-				show_valid_bit = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) {
-			show_cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) {
-			show_deleted = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) {
-			show_modified = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) {
-			show_others = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
-			dir.show_ignored = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) {
-			show_stage = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
-			show_killed = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--directory")) {
-			dir.show_other_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-empty-directory")) {
-			dir.hide_empty_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
-			/* There's no point in showing unmerged unless
-			 * you also show the stage information.
-			 */
-			show_stage = 1;
-			show_unmerged = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x") && i+1 < argc) {
-			exc_given = 1;
-			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			exc_given = 1;
-			add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strcmp(arg, "-X") && i+1 < argc) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, argv[++i]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-from=")) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, arg+15);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-per-directory=")) {
-			exc_given = 1;
-			dir.exclude_per_dir = arg + 24;
-			continue;
-		}
-		if (!strcmp(arg, "--exclude-standard")) {
-			exc_given = 1;
-			setup_standard_excludes(&dir);
-			continue;
-		}
-		if (!strcmp(arg, "--full-name")) {
-			prefix_offset = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--error-unmatch")) {
-			error_unmatch = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--with-tree=")) {
-			with_tree = arg + 12;
-			continue;
-		}
-		if (!prefixcmp(arg, "--abbrev=")) {
-			abbrev = strtoul(arg+9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (abbrev > 40)
-				abbrev = 40;
-			continue;
-		}
-		if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-			continue;
-		}
-		if (*arg == '-')
-			usage(ls_files_usage);
-		break;
+	argc = parse_options(argc, argv, builtin_ls_files_options,
+			ls_files_usage, 0);
+	if (show_tag || show_valid_bit) {
+		tag_cached = "H ";
+		tag_unmerged = "M ";
+		tag_removed = "R ";
+		tag_modified = "C ";
+		tag_other = "? ";
+		tag_killed = "K ";
 	}
+	if (show_modified || show_others || dir.show_ignored || show_killed)
+		require_work_tree = 1;
+	if (show_unmerged)
+		/* There's no point in showing unmerged unless
+		 * you also show the stage information.
+		 */
+		show_stage = 1;
+	if (dir.exclude_per_dir)
+		exc_given = 1;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, argv);
 
 	/* be nice with submodule patsh ending in a slash */
 	read_cache();
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] parse-opt: migrate builtin-ls-files.
  2009-02-15 19:54           ` [PATCH] " Miklos Vajna
@ 2009-02-15 20:13             ` Johannes Schindelin
  2009-02-15 22:07               ` Miklos Vajna
  0 siblings, 1 reply; 19+ messages in thread
From: Johannes Schindelin @ 2009-02-15 20:13 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, Pierre Habouzit, git

Hi,

On Sun, 15 Feb 2009, Miklos Vajna wrote:

> On Sat, Feb 14, 2009 at 09:56:04PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 8 Jan 2009, Miklos Vajna wrote:
> >
> > > +static int option_parse_ignored(const struct option *opt,
> > > +                           const char *arg, int unset)
> > > +{
> > > +   struct dir_struct *dir = opt->value;
> > > +
> > > +   dir->show_ignored = !unset;
> > > +
> > > +   return 0;
> > > +}
> >
> > Maybe this wants to be converted into an OPTION_BIT compatible data 
> > type?
> 
> I think that's not possible, as show_ignored is a bitfield.

Well, in my juvenile folly I dreamt of turning this into a proper 
bitfield...

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] parse-opt: migrate builtin-ls-files.
  2009-02-15 20:13             ` Johannes Schindelin
@ 2009-02-15 22:07               ` Miklos Vajna
  2009-02-16 12:20                 ` [PATCH] Turn the flags in struct dir_struct into a single variable Johannes Schindelin
  0 siblings, 1 reply; 19+ messages in thread
From: Miklos Vajna @ 2009-02-15 22:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Pierre Habouzit, git

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Sun, Feb 15, 2009 at 09:13:42PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > +   dir->show_ignored = !unset;
> > > > +
> > > > +   return 0;
> > > > +}
> > >
> > > Maybe this wants to be converted into an OPTION_BIT compatible data 
> > > type?
> > 
> > I think that's not possible, as show_ignored is a bitfield.
> 
> Well, in my juvenile folly I dreamt of turning this into a proper 
> bitfield...

Uhm, I think I haven't got what you mean. ;-)

Personally I would just remove all problematic bitfields in struct
dir_struct as I think it does not worth using them, but getting rid of
them just because of parseopt sounds insane as well - so I sended up
writing callback functions, as you saw in the patch.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] Turn the flags in struct dir_struct into a single variable
  2009-02-15 22:07               ` Miklos Vajna
@ 2009-02-16 12:20                 ` Johannes Schindelin
  2009-02-16 21:47                   ` Miklos Vajna
  2009-02-17 14:27                   ` [PATCH] parse-opt: migrate builtin-ls-files Miklos Vajna
  0 siblings, 2 replies; 19+ messages in thread
From: Johannes Schindelin @ 2009-02-16 12:20 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, Pierre Habouzit, git

By having flags represented as bits in the new member variable 'flags',
it will be easier to use parse_options when dir_struct is involved.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sun, 15 Feb 2009, Miklos Vajna wrote:

	> On Sun, Feb 15, 2009 at 09:13:42PM +0100, Johannes Schindelin
	> <Johannes.Schindelin@gmx.de> wrote:
	>
	> > Well, in my juvenile folly I dreamt of turning this into a proper 
	> > bitfield...
	> 
	> Uhm, I think I haven't got what you mean. ;-)

	I meant this...

 builtin-add.c       |    2 +-
 builtin-checkout.c  |    2 +-
 builtin-clean.c     |    4 ++--
 builtin-ls-files.c  |   14 ++++++++------
 builtin-merge.c     |    2 +-
 builtin-read-tree.c |    2 +-
 dir.c               |   17 +++++++++--------
 dir.h               |   12 +++++++-----
 wt-status.c         |    7 +++----
 9 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 7a5a8c7..f2066a1 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -106,7 +106,7 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
 	/* Set up the default git porcelain excludes */
 	memset(dir, 0, sizeof(*dir));
 	if (!ignored_too) {
-		dir->collect_ignored = 1;
+		dir->flags |= DIR_COLLECT_IGNORED;
 		setup_standard_excludes(dir);
 	}
 
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 20b34ce..5b4921d 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -405,7 +405,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.verbose_update = !opts->quiet;
 		topts.fn = twoway_merge;
 		topts.dir = xcalloc(1, sizeof(*topts.dir));
-		topts.dir->show_ignored = 1;
+		topts.dir->flags |= DIR_SHOW_IGNORED;
 		topts.dir->exclude_per_dir = ".gitignore";
 		tree = parse_tree_indirect(old->commit->object.sha1);
 		init_tree_desc(&trees[0], tree->buffer, tree->size);
diff --git a/builtin-clean.c b/builtin-clean.c
index f78c2fb..c5ad33d 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -60,7 +60,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	memset(&dir, 0, sizeof(dir));
 	if (ignored_only)
-		dir.show_ignored = 1;
+		dir.flags |= DIR_SHOW_IGNORED;
 
 	if (ignored && ignored_only)
 		die("-x and -X cannot be used together");
@@ -69,7 +69,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		die("clean.requireForce%s set and -n or -f not given; "
 		    "refusing to clean", config_set ? "" : " not");
 
-	dir.show_other_directories = 1;
+	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
 
 	if (!ignored)
 		setup_standard_excludes(&dir);
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 15f4e97..beb66b7 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -179,7 +179,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 		for (i = 0; i < active_nr; i++) {
 			struct cache_entry *ce = active_cache[i];
 			int dtype = ce_to_dtype(ce);
-			if (excluded(dir, ce->name, &dtype) != dir->show_ignored)
+			if (excluded(dir, ce->name, &dtype) !=
+					!!(dir->flags & DIR_SHOW_IGNORED))
 				continue;
 			if (show_unmerged && !ce_stage(ce))
 				continue;
@@ -194,7 +195,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 			struct stat st;
 			int err;
 			int dtype = ce_to_dtype(ce);
-			if (excluded(dir, ce->name, &dtype) != dir->show_ignored)
+			if (excluded(dir, ce->name, &dtype) !=
+					!!(dir->flags & DIR_SHOW_IGNORED))
 				continue;
 			if (ce->ce_flags & CE_UPDATE)
 				continue;
@@ -437,7 +439,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
-			dir.show_ignored = 1;
+			dir.flags |= DIR_SHOW_IGNORED;
 			require_work_tree = 1;
 			continue;
 		}
@@ -455,11 +457,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--directory")) {
-			dir.show_other_directories = 1;
+			dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
 			continue;
 		}
 		if (!strcmp(arg, "--no-empty-directory")) {
-			dir.hide_empty_directories = 1;
+			dir.flags |= DIR_HIDE_EMPTY_DIRECTORIES;
 			continue;
 		}
 		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
@@ -551,7 +553,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		ps_matched = xcalloc(1, num);
 	}
 
-	if (dir.show_ignored && !exc_given) {
+	if ((dir.flags & DIR_SHOW_IGNORED) && !exc_given) {
 		fprintf(stderr, "%s: --ignored needs some exclude pattern\n",
 			argv[0]);
 		exit(1);
diff --git a/builtin-merge.c b/builtin-merge.c
index 6d2160d..4c11935 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -636,7 +636,7 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote)
 	memset(&opts, 0, sizeof(opts));
 	memset(&t, 0, sizeof(t));
 	memset(&dir, 0, sizeof(dir));
-	dir.show_ignored = 1;
+	dir.flags |= DIR_SHOW_IGNORED;
 	dir.exclude_per_dir = ".gitignore";
 	opts.dir = &dir;
 
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 38fef34..8e02738 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -170,7 +170,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 				die("more than one --exclude-per-directory are given.");
 
 			dir = xcalloc(1, sizeof(*opts.dir));
-			dir->show_ignored = 1;
+			dir->flags |= DIR_SHOW_IGNORED;
 			dir->exclude_per_dir = arg + 24;
 			opts.dir = dir;
 			/* We do not need to nor want to do read-directory
diff --git a/dir.c b/dir.c
index b9230c4..56e386b 100644
--- a/dir.c
+++ b/dir.c
@@ -487,14 +487,14 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
 		return recurse_into_directory;
 
 	case index_gitdir:
-		if (dir->show_other_directories)
+		if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
 			return ignore_directory;
 		return show_directory;
 
 	case index_nonexistent:
-		if (dir->show_other_directories)
+		if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
 			break;
-		if (!dir->no_gitlinks) {
+		if (!(dir->flags & DIR_NO_GITLINKS)) {
 			unsigned char sha1[20];
 			if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
 				return show_directory;
@@ -503,7 +503,7 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
 	}
 
 	/* This is the "show_other_directories" case */
-	if (!dir->hide_empty_directories)
+	if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
 		return show_directory;
 	if (!read_directory_recursive(dir, dirname, dirname, len, 1, simplify))
 		return ignore_directory;
@@ -601,7 +601,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 
 			dtype = DTYPE(de);
 			exclude = excluded(dir, fullname, &dtype);
-			if (exclude && dir->collect_ignored
+			if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
 			    && in_pathspec(fullname, baselen + len, simplify))
 				dir_add_ignored(dir, fullname, baselen + len);
 
@@ -609,7 +609,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			 * Excluded? If we don't explicitly want to show
 			 * ignored files, ignore it
 			 */
-			if (exclude && !dir->show_ignored)
+			if (exclude && !(dir->flags & DIR_SHOW_IGNORED))
 				continue;
 
 			if (dtype == DT_UNKNOWN)
@@ -621,7 +621,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			 * even if we don't ignore them, since the
 			 * directory may contain files that we do..
 			 */
-			if (!exclude && dir->show_ignored) {
+			if (!exclude && (dir->flags & DIR_SHOW_IGNORED)) {
 				if (dtype != DT_DIR)
 					continue;
 			}
@@ -634,7 +634,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 				len++;
 				switch (treat_directory(dir, fullname, baselen + len, simplify)) {
 				case show_directory:
-					if (exclude != dir->show_ignored)
+					if (exclude != !!(dir->flags
+							& DIR_SHOW_IGNORED))
 						continue;
 					break;
 				case recurse_into_directory:
diff --git a/dir.h b/dir.h
index 8009877..811e865 100644
--- a/dir.h
+++ b/dir.h
@@ -34,11 +34,13 @@ struct exclude_stack {
 struct dir_struct {
 	int nr, alloc;
 	int ignored_nr, ignored_alloc;
-	unsigned int show_ignored:1,
-		     show_other_directories:1,
-		     hide_empty_directories:1,
-		     no_gitlinks:1,
-		     collect_ignored:1;
+	enum {
+		DIR_SHOW_IGNORED = 1<<0,
+		DIR_SHOW_OTHER_DIRECTORIES = 1<<1,
+		DIR_HIDE_EMPTY_DIRECTORIES = 1<<2,
+		DIR_NO_GITLINKS = 1<<3,
+		DIR_COLLECT_IGNORED = 1<<4
+	} flags;
 	struct dir_entry **entries;
 	struct dir_entry **ignored;
 
diff --git a/wt-status.c b/wt-status.c
index 96ff2f8..f2eae20 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -250,10 +250,9 @@ static void wt_status_print_untracked(struct wt_status *s)
 
 	memset(&dir, 0, sizeof(dir));
 
-	if (!s->untracked) {
-		dir.show_other_directories = 1;
-		dir.hide_empty_directories = 1;
-	}
+	if (!s->untracked)
+		dir.flags |=
+			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
 	setup_standard_excludes(&dir);
 
 	read_directory(&dir, ".", "", 0, NULL);
-- 
1.6.2.rc0.392.g7c2ec

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Turn the flags in struct dir_struct into a single variable
  2009-02-16 12:20                 ` [PATCH] Turn the flags in struct dir_struct into a single variable Johannes Schindelin
@ 2009-02-16 21:47                   ` Miklos Vajna
  2009-02-17 14:27                   ` [PATCH] parse-opt: migrate builtin-ls-files Miklos Vajna
  1 sibling, 0 replies; 19+ messages in thread
From: Miklos Vajna @ 2009-02-16 21:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Pierre Habouzit, git

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On Mon, Feb 16, 2009 at 01:20:25PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> By having flags represented as bits in the new member variable 'flags',
> it will be easier to use parse_options when dir_struct is involved.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	On Sun, 15 Feb 2009, Miklos Vajna wrote:
> 
> 	> On Sun, Feb 15, 2009 at 09:13:42PM +0100, Johannes Schindelin
> 	> <Johannes.Schindelin@gmx.de> wrote:
> 	>
> 	> > Well, in my juvenile folly I dreamt of turning this into a proper 
> 	> > bitfield...
> 	> 
> 	> Uhm, I think I haven't got what you mean. ;-)
> 
> 	I meant this...

Aah. That definitely makes the parseopt code shorter, thanks. :-)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] parse-opt: migrate builtin-ls-files.
  2009-02-16 12:20                 ` [PATCH] Turn the flags in struct dir_struct into a single variable Johannes Schindelin
  2009-02-16 21:47                   ` Miklos Vajna
@ 2009-02-17 14:27                   ` Miklos Vajna
  1 sibling, 0 replies; 19+ messages in thread
From: Miklos Vajna @ 2009-02-17 14:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Pierre Habouzit, git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-ls-files.c |  255 ++++++++++++++++++++++++---------------------------
 1 files changed, 120 insertions(+), 135 deletions(-)

On Mon, Feb 16, 2009 at 01:20:25PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 	I meant this...

And here is my patch on top of this one.
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index d36f80b..1742c0f 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -10,6 +10,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "tree.h"
+#include "parse-options.h"
 
 static int abbrev;
 static int show_deleted;
@@ -28,6 +29,7 @@ static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
+static int exc_given;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -376,156 +378,139 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_
 	return errors;
 }
 
-static const char ls_files_usage[] =
-	"git ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
-	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
-	"[ --exclude-per-directory=<filename> ] [--exclude-standard] "
-	"[--full-name] [--abbrev] [--] [<file>]*";
+static const char * const ls_files_usage[] = {
+	"git ls-files [options] [<file>]*",
+	NULL
+};
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	line_terminator = unset ? '\n' : '\0';
+
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	struct exclude_list *list = opt->value;
+
+	exc_given = 1;
+	add_exclude(arg, "", 0, list);
+
+	return 0;
+}
+
+static int option_parse_exclude_from(const struct option *opt,
+				     const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	add_excludes_from_file(dir, arg);
+
+	return 0;
+}
+
+static int option_parse_exclude_standard(const struct option *opt,
+					 const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+
+	exc_given = 1;
+	setup_standard_excludes(dir);
+
+	return 0;
+}
 
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int exc_given = 0, require_work_tree = 0;
+	int require_work_tree = 0, show_tag = 0;
 	struct dir_struct dir;
+	struct option builtin_ls_files_options[] = {
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_BOOLEAN('t', NULL, &show_tag,
+			"identify the file status with tags"),
+		OPT_BOOLEAN('v', NULL, &show_valid_bit,
+			"use lowercase letters for 'assume unchanged' files"),
+		OPT_BOOLEAN('c', "cached", &show_cached,
+			"show cached files in the output (default)"),
+		OPT_BOOLEAN('d', "deleted", &show_deleted,
+			"show deleted files in the output"),
+		OPT_BOOLEAN('m', "modified", &show_modified,
+			"show modified files in the output"),
+		OPT_BOOLEAN('o', "others", &show_others,
+			"show other files in the output"),
+		OPT_BIT('i', "ignored", &dir.flags,
+			"show ignored files in the output",
+			DIR_SHOW_IGNORED),
+		OPT_BOOLEAN('s', "stage", &show_stage,
+			"show staged contents' object name in the output"),
+		OPT_BOOLEAN('k', "killed", &show_killed,
+			"show files on the filesystem that need to be removed"),
+		OPT_BIT(0, "directory", &dir.flags,
+			"show 'other' directories' name only",
+			DIR_SHOW_OTHER_DIRECTORIES),
+		OPT_BIT(0, "empty-directory", &dir.flags,
+			"list empty directories",
+			DIR_HIDE_EMPTY_DIRECTORIES),
+		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
+			"show unmerged files in the output"),
+		{ OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], "pattern",
+			"skip files matching pattern",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 'X', "exclude-from", &dir, "file",
+			"exclude patterns are read from <file>",
+			0, option_parse_exclude_from },
+		OPT_STRING(0, "exclude-per-directory", &dir.exclude_per_dir, "file",
+			"read additional per-directory exclude patterns in <file>"),
+		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
+			"add the standard git exclusions",
+			PARSE_OPT_NOARG, option_parse_exclude_standard },
+		{ OPTION_SET_INT, 0, "full-name", &prefix_offset, NULL,
+			"make the output relative to the project top directory",
+			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+		OPT_BOOLEAN(0, "error-unmatch", &error_unmatch,
+			"if any <file> is not in the index, treat this as an error"),
+		OPT_STRING(0, "with-tree", &with_tree, "tree-ish",
+			"pretend that paths removed since <tree-ish> are still present"),
+		OPT__ABBREV(&abbrev),
+		OPT_END()
+	};
 
 	memset(&dir, 0, sizeof(dir));
 	if (prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config, NULL);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_terminator = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) {
-			tag_cached = "H ";
-			tag_unmerged = "M ";
-			tag_removed = "R ";
-			tag_modified = "C ";
-			tag_other = "? ";
-			tag_killed = "K ";
-			if (arg[1] == 'v')
-				show_valid_bit = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) {
-			show_cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) {
-			show_deleted = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) {
-			show_modified = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) {
-			show_others = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
-			dir.flags |= DIR_SHOW_IGNORED;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) {
-			show_stage = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
-			show_killed = 1;
-			require_work_tree = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--directory")) {
-			dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
-			continue;
-		}
-		if (!strcmp(arg, "--no-empty-directory")) {
-			dir.flags |= DIR_HIDE_EMPTY_DIRECTORIES;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
-			/* There's no point in showing unmerged unless
-			 * you also show the stage information.
-			 */
-			show_stage = 1;
-			show_unmerged = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x") && i+1 < argc) {
-			exc_given = 1;
-			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			exc_given = 1;
-			add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strcmp(arg, "-X") && i+1 < argc) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, argv[++i]);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-from=")) {
-			exc_given = 1;
-			add_excludes_from_file(&dir, arg+15);
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude-per-directory=")) {
-			exc_given = 1;
-			dir.exclude_per_dir = arg + 24;
-			continue;
-		}
-		if (!strcmp(arg, "--exclude-standard")) {
-			exc_given = 1;
-			setup_standard_excludes(&dir);
-			continue;
-		}
-		if (!strcmp(arg, "--full-name")) {
-			prefix_offset = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--error-unmatch")) {
-			error_unmatch = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--with-tree=")) {
-			with_tree = arg + 12;
-			continue;
-		}
-		if (!prefixcmp(arg, "--abbrev=")) {
-			abbrev = strtoul(arg+9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (abbrev > 40)
-				abbrev = 40;
-			continue;
-		}
-		if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-			continue;
-		}
-		if (*arg == '-')
-			usage(ls_files_usage);
-		break;
+	argc = parse_options(argc, argv, builtin_ls_files_options,
+			ls_files_usage, 0);
+	if (show_tag || show_valid_bit) {
+		tag_cached = "H ";
+		tag_unmerged = "M ";
+		tag_removed = "R ";
+		tag_modified = "C ";
+		tag_other = "? ";
+		tag_killed = "K ";
 	}
+	if (show_modified || show_others || (dir.flags & DIR_SHOW_IGNORED) || show_killed)
+		require_work_tree = 1;
+	if (show_unmerged)
+		/*
+		 * There's no point in showing unmerged unless
+		 * you also show the stage information.
+		 */
+		show_stage = 1;
+	if (dir.exclude_per_dir)
+		exc_given = 1;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, argv);
 
 	/* be nice with submodule patsh ending in a slash */
 	read_cache();
-- 
1.6.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2009-02-17 14:28 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-06  0:11 [PATCH] parse-opt: migrate builtin-ls-files Miklos Vajna
2009-01-06 10:22 ` Pierre Habouzit
2009-01-07  3:11   ` [PATCH v2] " Miklos Vajna
2009-01-07 14:46     ` Pierre Habouzit
2009-01-08  0:55       ` [PATCH v3] " Miklos Vajna
2009-01-08  1:10         ` Johannes Schindelin
2009-01-08  1:54           ` Miklos Vajna
2009-01-15  0:14         ` Miklos Vajna
2009-01-15  3:16           ` Junio C Hamano
2009-01-15  3:48             ` Miklos Vajna
2009-02-14 12:16             ` Miklos Vajna
2009-02-14 20:04               ` Junio C Hamano
2009-02-14 20:56         ` Johannes Schindelin
2009-02-15 19:54           ` [PATCH] " Miklos Vajna
2009-02-15 20:13             ` Johannes Schindelin
2009-02-15 22:07               ` Miklos Vajna
2009-02-16 12:20                 ` [PATCH] Turn the flags in struct dir_struct into a single variable Johannes Schindelin
2009-02-16 21:47                   ` Miklos Vajna
2009-02-17 14:27                   ` [PATCH] parse-opt: migrate builtin-ls-files Miklos Vajna

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.