Heiko Voigt <hvo...@hvoigt.net> writes:

> I did not know that you prefer a space after the function name. I simply
> imitated the style from C and there we do not have spaces. It makes the
> style rules a bit more complicated. Wouldn't it be nicer to have the
> same as in C so we have less rules?

I do not think so, as they are different languages.  The call site
of C functions have name and opening parenthesis without a SP in
between.  The call site of shell functions do not even have
parentheses.

In any case, personal preferences (including mine) do not matter
much, as there is no "this is scientificly superiour" in styles.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index aac575e..48014f2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -109,7 +109,8 @@ resolve_relative_url ()
>  #
>  module_list()
>  {
> -     git ls-files --error-unmatch --stage -- "$@" |
> +     (git ls-files --error-unmatch --stage -- "$@" ||
> +             echo '160000 0000000000000000000000000000000000000000 0 ') |

Is there a reason why the sentinel has to have the same mode pattern
as a GITLINK entry, NULL SHA-1, stage #0?  Or is the "path" being
empty all that matters?

Ah, OK, you did not want to touch the perl script downstream.  I
would have preferred a comment to document that, i.e.

        (
                git ls-files --error-unmatch --stage -- "$@" ||
                # an entry with an empty $path used as a signal
                echo '160000 0.... 0 '
        ) |
        perl -e '...

or

        (
                git ls-files --error-unmatch --stage -- "$@" ||
                echo 'unmatched pathspec exists'
        ) |
        perl -e '
        ...
        while (<STDIN>) {
                if (/^unmatched pathspec/) {
                        print;
                        next;
                }
                chomp;
                my ($mode, $sha1, $stage, $path) = ...

Either way, the reader of the code would not have to scratch her
head that way.

>       perl -e '
>       my %unmerged = ();
>       my ($null_sha1) = ("0" x 40);
> @@ -385,6 +386,10 @@ cmd_foreach()
>       module_list |
>       while read mode sha1 stage sm_path
>       do
> +             if test -z "$sm_path"; then
> +                     exit 1

Style:

        if test -z "$sm_path"
        then
                exit 1

I know module_list would have said "error: pathspec 'no-such' did
not match any file(s) known to git.  Did you forget to git add"
already, but because that comes at the very end of the input to the
filter written in perl (and with the way the filter is coded, it
will stay at the end), I am not sure if the user would notice it if
we exit like this.  By the time we hit this exit, we would have seen
"Entering $sm_path..." followed by whatever message given while in
the submodule for all the submodules that comes out of module_list,
no?

How about doing it this way to avoid that issue, to make sure we die
immediately after the typo is diagnosed without touching anything?

 git-submodule.sh | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 3aa7644..3f99d71 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -109,26 +109,47 @@ resolve_relative_url ()
 #
 module_list()
 {
-       git ls-files --error-unmatch --stage -- "$@" |
+       (
+               git ls-files --error-unmatch --stage -- "$@" ||
+               echo "unmatched pathspec exists"
+       ) |
        perl -e '
        my %unmerged = ();
        my ($null_sha1) = ("0" x 40);
+       my @out = ();
+       my $unmatched = 0;
        while (<STDIN>) {
+               if (/^unmatched pathspec/) {
+                       $unmatched = 1;
+                       next;
+               }
                chomp;
                my ($mode, $sha1, $stage, $path) =
                        /^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
                next unless $mode eq "160000";
                if ($stage ne "0") {
                        if (!$unmerged{$path}++) {
-                               print "$mode $null_sha1 U\t$path\n";
+                               push @out, "$mode $null_sha1 U\t$path\n";
                        }
                        next;
                }
-               print "$_\n";
+               push @out, "$_\n";
+       }
+       if ($unmatched) {
+               unshift @out, "#unmatched\n";
        }
+       print for (@out);
        '
 }
 
+check_unmatched ()
+{
+       if test "$1" = "#unmatched"
+       then
+               exit 1
+       fi
+}
+
 #
 # Map submodule path to submodule name
 #
@@ -385,6 +406,7 @@ cmd_foreach()
        module_list |
        while read mode sha1 stage sm_path
        do
+               check_unmatched "$mode"
                if test -e "$sm_path"/.git
                then
                        say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
@@ -437,6 +459,7 @@ cmd_init()
        module_list "$@" |
        while read mode sha1 stage sm_path
        do
+               check_unmatched "$mode"
                name=$(module_name "$sm_path") || exit
 
                # Copy url setting when it is not set yet
@@ -537,6 +560,7 @@ cmd_update()
        err=
        while read mode sha1 stage sm_path
        do
+               check_unmatched "$mode"
                if test "$stage" = U
                then
                        echo >&2 "Skipping unmerged submodule $sm_path"
@@ -932,6 +956,7 @@ cmd_status()
        module_list "$@" |
        while read mode sha1 stage sm_path
        do
+               check_unmatched "$mode"
                name=$(module_name "$sm_path") || exit
                url=$(git config submodule."$name".url)
                displaypath="$prefix$sm_path"
@@ -1000,6 +1025,7 @@ cmd_sync()
        module_list "$@" |
        while read mode sha1 stage sm_path
        do
+               check_unmatched "$mode"
                name=$(module_name "$sm_path")
                url=$(git config -f .gitmodules --get submodule."$name".url)
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to