Re* git submodule output on invalid command
Pieter de Bie <pdebie@ai.rug.nl> writes:
Show 8 quoted lines
> ..., 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.
Show 11 quoted lines
> 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 "$@"; thenShell 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)
