Re: [PATCH 2/2] ls-files: fix broken --no-empty-directory

[Jeff King <peff@peff.net>]

On Sun, Mar 08, 2009 at 10:13:12PM +0100, Miklos Vajna wrote:

> > +		OPT_BIT(0, "no-empty-directory", &dir.flags,
> > +			"don't show empty directories",
> >  			DIR_HIDE_EMPTY_DIRECTORIES),
> >  		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
> >  			"show unmerged files in the output"),
> 
> Thanks for catching this. But then why not using PARSE_OPT_NONEG?
> 
> That would avoid --no-no-empty-directory.

I think either we don't care about negation, in which case it is not
hurting anybody to support --no-no-empty-directory, or we do, in which
case you actually want to do the negation properly. Which would be
something like:

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 437c366..2c5f7a5 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -427,6 +427,7 @@ static int option_parse_exclude_standard(const struct option *opt,
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
 	int require_work_tree = 0, show_tag = 0;
+	int show_empty_directories = 1;
 	struct dir_struct dir;
 	struct option builtin_ls_files_options[] = {
 		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
@@ -454,9 +455,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "directory", &dir.flags,
 			"show 'other' directories' name only",
 			DIR_SHOW_OTHER_DIRECTORIES),
-		OPT_BIT(0, "no-empty-directory", &dir.flags,
-			"don't show empty directories",
-			DIR_HIDE_EMPTY_DIRECTORIES),
+		OPT_BOOLEAN(0, "empty-directory", &show_empty_directories,
+			"show empty directories (on by default)"),
 		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
 			"show unmerged files in the output"),
 		{ OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], "pattern",
@@ -506,6 +506,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		show_stage = 1;
 	if (dir.exclude_per_dir)
 		exc_given = 1;
+	if (!show_empty_directories)
+		dir.flags |= DIR_HIDE_EMPTY_DIRECTORIES;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();


which is even still a little confusing, as you get "--empty-directory"
in the usage message. But you would almost never want to use that, as it
is already the default.

-Peff