Re* git submodule output on invalid command

[Junio C Hamano <gitster@pobox.com>]

Pieter de Bie <pdebie@ai.rug.nl> writes:

> ..., something like 'git
> submodule satsus' is valid and should return nothing, because there are
> no submodules in the 'satsus' path. However, I still feel this should
> produce a warning.
>
> I'm sure there is a nicer way to alert the user than my patch below, which
> warns if the user did not supply any valid paths. Anyone else got a more
> satisfying approach?

"ls-files --error-unmatch" would warn you of mistyped nonexistent paths,
but "git submodule Makefile" would still catch the Makefile from the
toplevel superproject happily and will not complain without checking after
filtering by submodules.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 1c39b59..3aae746 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -59,7 +59,12 @@ resolve_relative_url ()
>  #
>  module_list()
>  {
> -       git ls-files --stage -- "$@" | grep '^160000 '
> +       git ls-files --stage -- "$@" | grep '^160000 ' ||
> +       if test -z "$@"; then

Shell nit; this must be "$*" not "$@", right?

> +               die "This repository contains no submodules"
> +       else
> +               die "Could not find any submodules in paths $@"

Because die() acts as if it is a fancier echo from output POV this does
not matter in practice, but as a principle, this should also be "$*"
instead.  Upon seeing "git submodule a b c", your intention is not to pass
three parameters "Could not find a", "b" and "c" to die() but is to pass a
single string that is the error message.

By the way, because this "limiting only to submodules" seem to appear very
often, we might want to give ls-files a native feature to do so, perhaps
something like this.  Then your warning/error could become:

	git ls-files --limit-type=submodule --error-unmatch

Although we would need to make the error message that comes from this
codepath tweakable by the caller.

 builtin-ls-files.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git c/builtin-ls-files.c w/builtin-ls-files.c
index e8d568e..69e3b5b 100644
--- c/builtin-ls-files.c
+++ w/builtin-ls-files.c
@@ -28,6 +28,10 @@ static const char **pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
+static unsigned int limit_types;
+#define ENTRY_REGULAR   02
+#define ENTRY_SYMLINK   01
+#define ENTRY_SUBMODULE 04
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -37,6 +41,18 @@ static const char *tag_killed = "";
 static const char *tag_modified = "";
 
 
+static unsigned int entry_type_from_name(const char *name)
+{
+	if (!strcmp(name, "regular"))
+		return ENTRY_REGULAR;
+	else if (!strcmp(name, "symlink"))
+		return ENTRY_SYMLINK;
+	else if (!strcmp(name, "submodule"))
+		return ENTRY_SUBMODULE;
+	else
+		die("Unknown entry type: %s", name);
+}
+
 /*
  * Match a pathspec against a filename. The first "skiplen" characters
  * are the common prefix
@@ -205,6 +221,20 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 		tag = alttag;
 	}
 
+	if (limit_types) {
+		unsigned int entry_type;
+		if (S_ISLNK(ce->ce_mode))
+			entry_type = ENTRY_SYMLINK;
+		else if (S_ISREG(ce->ce_mode))
+			entry_type = ENTRY_REGULAR;
+		else if (S_ISGITLINK(ce->ce_mode))
+			entry_type = ENTRY_SUBMODULE;
+		else
+			die("Unknown type of entry %06o", ce->ce_mode);
+		if (!(limit_types & entry_type))
+			return;
+	}
+
 	if (!show_stage) {
 		fputs(tag, stdout);
 	} else {
@@ -551,6 +581,10 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			with_tree = arg + 12;
 			continue;
 		}
+		if (!prefixcmp(arg, "--limit-type=")) {
+			limit_types |= entry_type_from_name(arg + 13);
+			continue;
+		}
 		if (!prefixcmp(arg, "--abbrev=")) {
 			abbrev = strtoul(arg+9, NULL, 10);
 			if (abbrev && abbrev < MINIMUM_ABBREV)