[PATCH] git-svn: loosen config globs limitations

[PATCH] git-svn: loosen config globs limitations

[Victor Leschuk]

Expand the area of globs applicability for branches and tags
in git-svn. It is now possible to use globs like 'a*e', or 'release_*'.
This allows users to avoid long lines in config like:

branches = branches/{release_20,release_21,release_22,...}

Signed-off-by: Victor Leschuk <vleschuk@accesssoftek.com>
---
Changes from $gmane/283172:
  * Not only prefixed globs are allowed now (one can put '*' in the middle of a word) - thus changed patch name
  * Added tests for globs in the middle of a word

 Documentation/git-svn.txt                  |  12 ++
 perl/Git/SVN/GlobSpec.pm                   |  12 +-
 t/t9168-git-svn-partially-globbed-names.sh | 222 +++++++++++++++++++++++++++++
 3 files changed, 241 insertions(+), 5 deletions(-)
 create mode 100755 t/t9168-git-svn-partially-globbed-names.sh

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 0c0f60b..fb23a98 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -1034,6 +1034,8 @@ listed below are allowed:
 	url = http://server.org/svn
 	fetch = trunk/project-a:refs/remotes/project-a/trunk
 	branches = branches/*/project-a:refs/remotes/project-a/branches/*
+	branches = branches/release_*:refs/remotes/project-a/branches/release_*
+	branches = branches/re*se:refs/remotes/project-a/branches/*
 	tags = tags/*/project-a:refs/remotes/project-a/tags/*
 ------------------------------------------------------------------------
 
@@ -1044,6 +1046,16 @@ independent path component (surrounded by '/' or EOL).   This
 type of configuration is not automatically created by 'init' and
 should be manually entered with a text-editor or using 'git config'.
 
+Also note that only one asterisk is allowed per word. For example:
+
+	branches = branches/re*se:refs/remotes/project-a/branches/*
+
+will match branches 'release', 'rese', 're123se', however
+
+	branches = branches/re*s*e:refs/remotes/project-a/branches/*
+
+will produce an error.
+
 It is also possible to fetch a subset of branches or tags by using a
 comma-separated list of names within braces. For example:
 
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index c95f5d7..1248e6d 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -11,16 +11,18 @@ sub new {
 	my $die_msg = "Only one set of wildcard directories " .
 				"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
 	for my $part (split(m|/|, $glob)) {
-		if ($part =~ /\*/ && $part ne "*") {
-			die "Invalid pattern in '$glob': $part\n";
-		} elsif ($pattern_ok && $part =~ /[{}]/ &&
+		if ($pattern_ok && $part =~ /[{}]/ &&
 			 $part !~ /^\{[^{}]+\}/) {
 			die "Invalid pattern in '$glob': $part\n";
 		}
-		if ($part eq "*") {
+		my $nstars = $part =~ tr/\*//;
+		die "Only one '*' is allowed in a pattern: '$part'\n" if $nstars > 1;
+		if ($part =~ /(.*)\*(.*)/) {
 			die $die_msg if $state eq "right";
+			my ($l, $r) = ($1, $2);
 			$state = "pattern";
-			push(@patterns, "[^/]*");
+			my $pat = quotemeta($l) . '[^/]*' . quotemeta($r);
+			push(@patterns, $pat);
 		} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
 			die $die_msg if $state eq "right";
 			$state = "pattern";
diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
new file mode 100755
index 0000000..8e4100a
--- /dev/null
+++ b/t/t9168-git-svn-partially-globbed-names.sh
@@ -0,0 +1,222 @@
+#!/bin/sh
+test_description='git svn globbing refspecs with prefixed globs'
+. ./lib-git-svn.sh
+
+test_expect_success 'prepare test refspec prefixed globbing' '
+	cat >expect.end <<EOF
+the end
+hi
+start a new branch
+initial
+EOF
+	'
+
+test_expect_success 'test refspec prefixed globbing' '
+	mkdir -p trunk/src/a trunk/src/b trunk/doc &&
+	echo "hello world" >trunk/src/a/readme &&
+	echo "goodbye world" >trunk/src/b/readme &&
+	svn_cmd import -m "initial" trunk "$svnrepo"/trunk &&
+	svn_cmd co "$svnrepo" tmp &&
+	(
+		cd tmp &&
+		mkdir branches tags &&
+		svn_cmd add branches tags &&
+		svn_cmd cp trunk branches/b_start &&
+		svn_cmd commit -m "start a new branch" &&
+		svn_cmd up &&
+		echo "hi" >>branches/b_start/src/b/readme &&
+		poke branches/b_start/src/b/readme &&
+		echo "hey" >>branches/b_start/src/a/readme &&
+		poke branches/b_start/src/a/readme &&
+		svn_cmd commit -m "hi" &&
+		svn_cmd up &&
+		svn_cmd cp branches/b_start tags/t_end &&
+		echo "bye" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		echo "aye" >>tags/t_end/src/a/readme &&
+		poke tags/t_end/src/a/readme &&
+		svn_cmd commit -m "the end" &&
+		echo "byebye" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		svn_cmd commit -m "nothing to see here"
+	) &&
+	git config --add svn-remote.svn.url "$svnrepo" &&
+	git config --add svn-remote.svn.fetch \
+	                 "trunk/src/a:refs/remotes/trunk" &&
+	git config --add svn-remote.svn.branches \
+	                 "branches/b_*/src/a:refs/remotes/branches/b_*" &&
+	git config --add svn-remote.svn.tags\
+	                 "tags/t_*/src/a:refs/remotes/tags/t_*" &&
+	git svn multi-fetch &&
+	git log --pretty=oneline refs/remotes/tags/t_end | \
+	    sed -e "s/^.\{41\}//" >output.end &&
+	test_cmp expect.end output.end &&
+	test "$(git rev-parse refs/remotes/tags/t_end~1)" = \
+		"$(git rev-parse refs/remotes/branches/b_start)" &&
+	test "$(git rev-parse refs/remotes/branches/b_start~2)" = \
+		"$(git rev-parse refs/remotes/trunk)" &&
+	test_must_fail git rev-parse refs/remotes/tags/t_end@3
+	'
+
+test_expect_success 'prepare test left-hand-side only prefixed globbing' '
+	echo try to try >expect.two &&
+	echo nothing to see here >>expect.two &&
+	cat expect.end >>expect.two
+	'
+
+test_expect_success 'test left-hand-side only prefixed globbing' '
+	git config --add svn-remote.two.url "$svnrepo" &&
+	git config --add svn-remote.two.fetch trunk:refs/remotes/two/trunk &&
+	git config --add svn-remote.two.branches \
+	                 "branches/b_*:refs/remotes/two/branches/*" &&
+	git config --add svn-remote.two.tags \
+	                 "tags/t_*:refs/remotes/two/tags/*" &&
+	(
+		cd tmp &&
+		echo "try try" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		svn_cmd commit -m "try to try"
+	) &&
+	git svn fetch two &&
+	test $(git rev-list refs/remotes/two/tags/t_end | wc -l) -eq 6 &&
+	test $(git rev-list refs/remotes/two/branches/b_start | wc -l) -eq 3 &&
+	test $(git rev-parse refs/remotes/two/branches/b_start~2) = \
+	     $(git rev-parse refs/remotes/two/trunk) &&
+	test $(git rev-parse refs/remotes/two/tags/t_end~3) = \
+	     $(git rev-parse refs/remotes/two/branches/b_start) &&
+	git log --pretty=oneline refs/remotes/two/tags/t_end | \
+	    sed -e "s/^.\{41\}//" >output.two &&
+	test_cmp expect.two output.two
+	'
+
+test_expect_success 'prepare test prefixed globs match just prefix' '
+	cat >expect.three <<EOF
+Tag commit to t_
+Branch commit to b_
+initial
+EOF
+	'
+
+test_expect_success 'test prefixed globs match just prefix' '
+	git config --add svn-remote.three.url "$svnrepo" &&
+	git config --add svn-remote.three.fetch \
+	                 trunk:refs/remotes/three/trunk &&
+	git config --add svn-remote.three.branches \
+	                 "branches/b_*:refs/remotes/three/branches/*" &&
+	git config --add svn-remote.three.tags \
+	                 "tags/t_*:refs/remotes/three/tags/*" &&
+	(
+		cd tmp &&
+		svn_cmd cp trunk branches/b_ &&
+		echo "Branch commit to b_" >>branches/b_/src/a/readme &&
+		poke branches/b_/src/a/readme &&
+		svn_cmd commit -m "Branch commit to b_" &&
+		svn_cmd up && svn_cmd cp branches/b_ tags/t_ &&
+		echo "Tag commit to t_" >>tags/t_/src/a/readme &&
+		poke tags/t_/src/a/readme &&
+		svn_cmd commit -m "Tag commit to t_" &&
+		svn_cmd up
+	) &&
+	git svn fetch three &&
+	test $(git rev-list refs/remotes/three/branches/b_ | wc -l) -eq 2 &&
+	test $(git rev-list refs/remotes/three/tags/t_ | wc -l) -eq 3 &&
+	test $(git rev-parse refs/remotes/three/branches/b_~1) = \
+	     $(git rev-parse refs/remotes/three/trunk) &&
+	test $(git rev-parse refs/remotes/three/tags/t_~1) = \
+	     $(git rev-parse refs/remotes/three/branches/b_) &&
+	git log --pretty=oneline refs/remotes/three/tags/t_ | \
+	    sed -e "s/^.\{41\}//" >output.three &&
+	test_cmp expect.three output.three
+	'
+
+test_expect_success 'prepare test disallow prefixed multi-globs' "
+	echo \"Only one set of wildcard directories\" \
+	     \"(e.g. '*' or '*/*/*') is supported: 'branches/b_*/t/*'\" >expect.four &&
+	echo \"\" >>expect.four
+	"
+
+test_expect_success 'test disallow prefixed multi-globs' '
+	git config --add svn-remote.four.url "$svnrepo" &&
+	git config --add svn-remote.four.fetch \
+	                 trunk:refs/remotes/four/trunk &&
+	git config --add svn-remote.four.branches \
+	                 "branches/b_*/t/*:refs/remotes/four/branches/*" &&
+	git config --add svn-remote.four.tags \
+	                 "tags/t_*/*:refs/remotes/four/tags/*" &&
+	(
+		cd tmp &&
+		echo "try try" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		svn_cmd commit -m "try to try"
+	) &&
+	test_must_fail git svn fetch four 2>stderr.four &&
+	test_cmp expect.four stderr.four &&
+	git config --unset svn-remote.four.branches &&
+	git config --unset svn-remote.four.tags
+	'
+
+test_expect_success 'prepare test globbing in the middle of the word' '
+	cat >expect.five <<EOF
+Tag commit to fghij
+Branch commit to abcde
+initial
+EOF
+	'
+
+test_expect_success 'test globbing in the middle of the word' '
+	git config --add svn-remote.five.url "$svnrepo" &&
+	git config --add svn-remote.five.fetch \
+	                 trunk:refs/remotes/five/trunk &&
+	git config --add svn-remote.five.branches \
+	                 "branches/a*e:refs/remotes/five/branches/*" &&
+	git config --add svn-remote.five.tags \
+	                 "tags/f*j:refs/remotes/five/tags/*" &&
+	(
+		cd tmp &&
+		svn_cmd cp trunk branches/abcde &&
+		echo "Branch commit to abcde" >>branches/abcde/src/a/readme &&
+		poke branches/b_/src/a/readme &&
+		svn_cmd commit -m "Branch commit to abcde" &&
+		svn_cmd up &&
+		svn_cmd cp branches/abcde tags/fghij &&
+		echo "Tag commit to fghij" >>tags/fghij/src/a/readme &&
+		poke tags/fghij/src/a/readme &&
+		svn_cmd commit -m "Tag commit to fghij" &&
+		svn_cmd up
+	) &&
+	git svn fetch five &&
+	test $(git rev-list refs/remotes/five/branches/abcde | wc -l) -eq 2 &&
+	test $(git rev-list refs/remotes/five/tags/fghij | wc -l) -eq 3 &&
+	test $(git rev-parse refs/remotes/five/branches/abcde~1) = \
+	     $(git rev-parse refs/remotes/five/trunk) &&
+	test $(git rev-parse refs/remotes/five/tags/fghij~1) = \
+	     $(git rev-parse refs/remotes/five/branches/abcde) &&
+	git log --pretty=oneline refs/remotes/five/tags/fghij | \
+	    sed -e "s/^.\{41\}//" >output.five &&
+	test_cmp expect.five output.five
+	'
+
+test_expect_success 'prepare test disallow multiple asterisks in one word' "
+	echo \"Only one '*' is allowed in a pattern: 'a*c*e'\" >expect.six &&
+	echo \"\" >>expect.six
+	"
+
+test_expect_success 'test disallow multiple asterisks in one word' '
+	git config --add svn-remote.six.url "$svnrepo" &&
+	git config --add svn-remote.six.fetch \
+	                 trunk:refs/remotes/six/trunk &&
+	git config --add svn-remote.six.branches \
+	                 "branches/a*c*e:refs/remotes/six/branches/*" &&
+	git config --add svn-remote.six.tags \
+	                 "tags/f*h*j:refs/remotes/six/tags/*" &&
+	(
+		cd tmp &&
+		echo "try try" >>tags/fghij/src/b/readme &&
+		poke tags/fghij/src/b/readme &&
+		svn_cmd commit -m "try to try"
+	) &&
+	test_must_fail git svn fetch six 2>stderr.six &&
+	test_cmp expect.six stderr.six
+	'
+
+test_done
-- 
2.7.0.1.g72018be.dirty

Re: [PATCH] git-svn: loosen config globs limitations

[Eric Wong]

Thanks, I made a minor cleanup and applied with --whitespace=fix
to remove spaces from indentation.

--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -15,8 +15,10 @@ sub new {
 			 $part !~ /^\{[^{}]+\}/) {
 			die "Invalid pattern in '$glob': $part\n";
 		}
-		my $nstars = $part =~ tr/\*//;
-		die "Only one '*' is allowed in a pattern: '$part'\n" if $nstars > 1;
+		my $nstars = $part =~ tr/*//;
+		if ($nstars > 1) {
+			die "Only one '*' is allowed in a pattern: '$part'\n";
+		}
 		if ($part =~ /(.*)\*(.*)/) {
 			die $die_msg if $state eq "right";
 			my ($l, $r) = ($1, $2);

So I'll push out with the following commit message:

Subject: [PATCH] git-svn: loosen config globs limitations

Expand the area of globs applicability for branches and tags
in git-svn. It is now possible to use globs like 'a*e', or 'release_*'.
This allows users to avoid long lines in config like:

	branches = branches/{release_20,release_21,release_22,...}

In favor of:

	branches = branches/release_*

[ew: amended commit message, minor formatting and style fixes]

Signed-off-by: Victor Leschuk <vleschuk@accesssoftek.com>
Signed-off-by: Eric Wong <normalperson@yhbt.net>



I also noticed the "Only one set of wildcard directories" error
message is unnecessary long and "wildcard directories" should
probably be shortened to "wildcards" to avoid wrapping in a terminal.
That will probably be a separate patch for me.

RE: [PATCH] git-svn: loosen config globs limitations

[Victor Leschuk]

Thanks a lot Eric,

I agree with all corrections, I also noticed the "wildcard directories" message situation when was creating test for the patch, however didn't want mix up unrelated changes for this patchset.

--
Best Regards,
Victor
________________________________________
From: Eric Wong [normalperson@yhbt.net]
Sent: Tuesday, January 12, 2016 19:16
To: Victor Leschuk
Cc: git@vger.kernel.org; Victor Leschuk; gitster@pobox.com
Subject: Re: [PATCH] git-svn: loosen config globs limitations

Thanks, I made a minor cleanup and applied with --whitespace=fix
to remove spaces from indentation.

--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -15,8 +15,10 @@ sub new {
                         $part !~ /^\{[^{}]+\}/) {
                        die "Invalid pattern in '$glob': $part\n";
                }
-               my $nstars = $part =~ tr/\*//;
-               die "Only one '*' is allowed in a pattern: '$part'\n" if $nstars > 1;
+               my $nstars = $part =~ tr/*//;
+               if ($nstars > 1) {
+                       die "Only one '*' is allowed in a pattern: '$part'\n";
+               }
                if ($part =~ /(.*)\*(.*)/) {
                        die $die_msg if $state eq "right";
                        my ($l, $r) = ($1, $2);

So I'll push out with the following commit message:

Subject: [PATCH] git-svn: loosen config globs limitations

Expand the area of globs applicability for branches and tags
in git-svn. It is now possible to use globs like 'a*e', or 'release_*'.
This allows users to avoid long lines in config like:

        branches = branches/{release_20,release_21,release_22,...}

In favor of:

        branches = branches/release_*

[ew: amended commit message, minor formatting and style fixes]

Signed-off-by: Victor Leschuk <vleschuk@accesssoftek.com>
Signed-off-by: Eric Wong <normalperson@yhbt.net>



I also noticed the "Only one set of wildcard directories" error
message is unnecessary long and "wildcard directories" should
probably be shortened to "wildcards" to avoid wrapping in a terminal.
That will probably be a separate patch for me.

Re: [PATCH] git-svn: loosen config globs limitations

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Thanks, I made a minor cleanup and applied with --whitespace=fix
> to remove spaces from indentation.
> ...
> I also noticed the "Only one set of wildcard directories" error
> message is unnecessary long and "wildcard directories" should
> probably be shortened to "wildcards" to avoid wrapping in a terminal.
> That will probably be a separate patch for me.

Should I pull something from you now from 'master' at your
bogomips.org repository?  I do not mind (and actually I would
prefer) waiting until I hear a go ahead, which would let you work on
your own changes before I pull.

Thanks.

Re: [PATCH] git-svn: loosen config globs limitations

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> > I also noticed the "Only one set of wildcard directories" error
> > message is unnecessary long and "wildcard directories" should
> > probably be shortened to "wildcards" to avoid wrapping in a terminal.
> > That will probably be a separate patch for me.
> 
> Should I pull something from you now from 'master' at your
> bogomips.org repository?  I do not mind (and actually I would
> prefer) waiting until I hear a go ahead, which would let you work on
> your own changes before I pull.

Yes, please wait.  I'll fix the long messages later today.  Thanks

[PATCH] git-svn: shorten glob error message

[Eric Wong]

Error messages should attempt to fit within the confines of
an 80-column terminal to avoid compatibility and accessibility
problems.  Furthermore the word "directories" can be misleading
when used in the context of git refnames.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
  Eric Wong <normalperson@yhbt.net> wrote:
  > I also noticed the "Only one set of wildcard directories" error
  > message is unnecessary long and "wildcard directories" should
  > probably be shortened to "wildcards" to avoid wrapping in a terminal.
  > That will probably be a separate patch for me.

  There's likely more instances of this in git-svn, but I figured
  we'll get this one fixed, first.

  Also pushed to bogomips.org/git-svn.git
  (commit dc6aa7e61e9d33856f54d63b7acb518383420373)
  along with Victor's patch.

 perl/Git/SVN/GlobSpec.pm                   | 4 ++--
 t/t9108-git-svn-glob.sh                    | 9 ++++++---
 t/t9109-git-svn-multi-glob.sh              | 9 ++++++---
 t/t9168-git-svn-partially-globbed-names.sh | 7 ++++---
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index 4775026..a0a8d17 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -8,8 +8,8 @@ sub new {
 	$re =~ s!/+$!!g; # no need for trailing slashes
 	my (@left, @right, @patterns);
 	my $state = "left";
-	my $die_msg = "Only one set of wildcard directories " .
-				"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
+	my $die_msg = "Only one set of wildcards " .
+				"(e.g. '*' or '*/*/*') is supported: $glob\n";
 	for my $part (split(m|/|, $glob)) {
 		if ($pattern_ok && $part =~ /[{}]/ &&
 			 $part !~ /^\{[^{}]+\}/) {
diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
index d732d31..29b363b 100755
--- a/t/t9108-git-svn-glob.sh
+++ b/t/t9108-git-svn-glob.sh
@@ -86,9 +86,12 @@ test_expect_success 'test left-hand-side only globbing' '
 	test_cmp expect.two output.two
 	'
 
-echo "Only one set of wildcard directories" \
-     "(e.g. '*' or '*/*/*') is supported: 'branches/*/t/*'" > expect.three
-echo "" >> expect.three
+test_expect_success 'prepare test disallow multi-globs' "
+cat >expect.three <<EOF
+Only one set of wildcards (e.g. '*' or '*/*/*') is supported: branches/*/t/*
+
+EOF
+	"
 
 test_expect_success 'test disallow multi-globs' '
 	git config --add svn-remote.three.url "$svnrepo" &&
diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
index c318f9f..d0b79fe 100755
--- a/t/t9109-git-svn-multi-glob.sh
+++ b/t/t9109-git-svn-multi-glob.sh
@@ -135,9 +135,12 @@ test_expect_success 'test another branch' '
 	test_cmp expect.four output.four
 	'
 
-echo "Only one set of wildcard directories" \
-     "(e.g. '*' or '*/*/*') is supported: 'branches/*/t/*'" > expect.three
-echo "" >> expect.three
+test_expect_success 'prepare test disallow multiple globs' "
+cat >expect.three <<EOF
+Only one set of wildcards (e.g. '*' or '*/*/*') is supported: branches/*/t/*
+
+EOF
+	"
 
 test_expect_success 'test disallow multiple globs' '
 	git config --add svn-remote.three.url "$svnrepo" &&
diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
index a7641dc..8b22f22 100755
--- a/t/t9168-git-svn-partially-globbed-names.sh
+++ b/t/t9168-git-svn-partially-globbed-names.sh
@@ -130,9 +130,10 @@ test_expect_success 'test prefixed globs match just prefix' '
 	'
 
 test_expect_success 'prepare test disallow prefixed multi-globs' "
-	echo \"Only one set of wildcard directories\" \
-	     \"(e.g. '*' or '*/*/*') is supported: 'branches/b_*/t/*'\" >expect.four &&
-	echo \"\" >>expect.four
+cat >expect.four <<EOF
+Only one set of wildcards (e.g. '*' or '*/*/*') is supported: branches/b_*/t/*
+
+EOF
 	"
 
 test_expect_success 'test disallow prefixed multi-globs' '
-- 
EW

Re: [PATCH] git-svn: shorten glob error message

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Error messages should attempt to fit within the confines of
> an 80-column terminal to avoid compatibility and accessibility
> problems.  Furthermore the word "directories" can be misleading
> when used in the context of git refnames.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>   Eric Wong <normalperson@yhbt.net> wrote:
>   > I also noticed the "Only one set of wildcard directories" error
>   > message is unnecessary long and "wildcard directories" should
>   > probably be shortened to "wildcards" to avoid wrapping in a terminal.
>   > That will probably be a separate patch for me.
>
>   There's likely more instances of this in git-svn, but I figured
>   we'll get this one fixed, first.
>
>   Also pushed to bogomips.org/git-svn.git
>   (commit dc6aa7e61e9d33856f54d63b7acb518383420373)
>   along with Victor's patch.

Thanks.

I am not sure if it is a good idea to show */*/* as an example in
the message (that is an anti-example of 'one set of wildcard' by
having three stars, isn't it?), but that is not a new issue this
change introduces.

>  	my $state = "left";
> -	my $die_msg = "Only one set of wildcard directories " .
> -				"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
> +	my $die_msg = "Only one set of wildcards " .
> +				"(e.g. '*' or '*/*/*') is supported: $glob\n";
>  	for my $part (split(m|/|, $glob)) {
>  		if ($pattern_ok && $part =~ /[{}]/ &&
>  			 $part !~ /^\{[^{}]+\}/) {
> diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
> index d732d31..29b363b 100755
> --- a/t/t9108-git-svn-glob.sh
> +++ b/t/t9108-git-svn-glob.sh
> @@ -86,9 +86,12 @@ test_expect_success 'test left-hand-side only globbing' '
>  	test_cmp expect.two output.two
>  	'
>  
> -echo "Only one set of wildcard directories" \
> -     "(e.g. '*' or '*/*/*') is supported: 'branches/*/t/*'" > expect.three
> -echo "" >> expect.three
> +test_expect_success 'prepare test disallow multi-globs' "
> +cat >expect.three <<EOF
> +Only one set of wildcards (e.g. '*' or '*/*/*') is supported: branches/*/t/*
> +
> +EOF
> +	"
>  
>  test_expect_success 'test disallow multi-globs' '
>  	git config --add svn-remote.three.url "$svnrepo" &&
> diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
> index c318f9f..d0b79fe 100755
> --- a/t/t9109-git-svn-multi-glob.sh
> +++ b/t/t9109-git-svn-multi-glob.sh
> @@ -135,9 +135,12 @@ test_expect_success 'test another branch' '
>  	test_cmp expect.four output.four
>  	'
>  
> -echo "Only one set of wildcard directories" \
> -     "(e.g. '*' or '*/*/*') is supported: 'branches/*/t/*'" > expect.three
> -echo "" >> expect.three
> +test_expect_success 'prepare test disallow multiple globs' "
> +cat >expect.three <<EOF
> +Only one set of wildcards (e.g. '*' or '*/*/*') is supported: branches/*/t/*
> +
> +EOF
> +	"
>  
>  test_expect_success 'test disallow multiple globs' '
>  	git config --add svn-remote.three.url "$svnrepo" &&
> diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
> index a7641dc..8b22f22 100755
> --- a/t/t9168-git-svn-partially-globbed-names.sh
> +++ b/t/t9168-git-svn-partially-globbed-names.sh
> @@ -130,9 +130,10 @@ test_expect_success 'test prefixed globs match just prefix' '
>  	'
>  
>  test_expect_success 'prepare test disallow prefixed multi-globs' "
> -	echo \"Only one set of wildcard directories\" \
> -	     \"(e.g. '*' or '*/*/*') is supported: 'branches/b_*/t/*'\" >expect.four &&
> -	echo \"\" >>expect.four
> +cat >expect.four <<EOF
> +Only one set of wildcards (e.g. '*' or '*/*/*') is supported: branches/b_*/t/*
> +
> +EOF
>  	"
>  
>  test_expect_success 'test disallow prefixed multi-globs' '

Re: [PATCH] git-svn: shorten glob error message

[Victor Leschuk]

Hello all,

On 01/14/2016 09:15 PM, Junio C Hamano wrote:
> Eric Wong <normalperson@yhbt.net> writes:
>
>> Error messages should attempt to fit within the confines of
>> an 80-column terminal to avoid compatibility and accessibility
>> problems.  Furthermore the word "directories" can be misleading
>> when used in the context of git refnames.
>>
>> Signed-off-by: Eric Wong <normalperson@yhbt.net>
>> ---
>>    Eric Wong <normalperson@yhbt.net> wrote:
>>    > I also noticed the "Only one set of wildcard directories" error
>>    > message is unnecessary long and "wildcard directories" should
>>    > probably be shortened to "wildcards" to avoid wrapping in a terminal.
>>    > That will probably be a separate patch for me.
>>
>>    There's likely more instances of this in git-svn, but I figured
>>    we'll get this one fixed, first.
>>
>>    Also pushed to bogomips.org/git-svn.git
>>    (commit dc6aa7e61e9d33856f54d63b7acb518383420373)
>>    along with Victor's patch.
> Thanks.
>
> I am not sure if it is a good idea to show */*/* as an example in
> the message (that is an anti-example of 'one set of wildcard' by
> having three stars, isn't it?), but that is not a new issue this
> change introduces.

I agree, this should be changed, however I think this should be done in 
separate patch.

Do we have any questions left open before this could be merged into main 
git repo?

--
Victor

Re: [PATCH] git-svn: shorten glob error message

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> I am not sure if it is a good idea to show */*/* as an example in
> the message (that is an anti-example of 'one set of wildcard' by
> having three stars, isn't it?), but that is not a new issue this
> change introduces.

Actually, going back to commit 570d35c26dfbc40757da6032cdc96afb58cc0037
("git-svn: Allow deep branch names by supporting multi-globs"),
having equal '*' on both sides is all that is required.

Not sure how to improve the wording, though...

> >  	my $state = "left";
> > -	my $die_msg = "Only one set of wildcard directories " .
> > -				"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
> > +	my $die_msg = "Only one set of wildcards " .
> > +				"(e.g. '*' or '*/*/*') is supported: $glob\n";
> >  	for my $part (split(m|/|, $glob)) {
> >  		if ($pattern_ok && $part =~ /[{}]/ &&
> >  			 $part !~ /^\{[^{}]+\}/) {

Re: [PATCH] git-svn: shorten glob error message

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>> I am not sure if it is a good idea to show */*/* as an example in
>> the message (that is an anti-example of 'one set of wildcard' by
>> having three stars, isn't it?), but that is not a new issue this
>> change introduces.
>
> Actually, going back to commit 570d35c26dfbc40757da6032cdc96afb58cc0037
> ("git-svn: Allow deep branch names by supporting multi-globs"),
> having equal '*' on both sides is all that is required.
>
> Not sure how to improve the wording, though...

I dunno, either, and that is why "not a new issue", iow, the patch
is good as-is.  The wording might be an area with possible future
improvement, but that does not have to block the improvement the
patch under discussion brings us.

Thanks.