[BUG] git completion do sed on binary file

[BUG] git completion do sed on binary file

[Nguyen Thai Ngoc Duy]

Probably missed since git-merge builtin effort:

__git_merge_strategies ()
{
        if [ -n "$__git_merge_strategylist" ]; then
                echo "$__git_merge_strategylist"
                return
        fi
        sed -n "/^all_strategies='/{
                s/^all_strategies='//
                s/'//
                p
                q
                }" "$(git --exec-path)/git-merge"
}

It takes several seconds to finish that function.
-- 
Duy

[PATCH] bash-completion: fix getting strategy list

[Nguyen Thai Ngoc Duy]

Bash completion needs to know what strategies git supports. Maybe
other similar tools have the same demand. So add 
"git merge--show-strategies"

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---

  On Tue, Aug 19, 2008 at 07:27:39PM +0700, Nguyen Thai Ngoc Duy wrote:
  > Probably missed since git-merge builtin effort:
  > 
  > __git_merge_strategies ()
  > {
  >         if [ -n "$__git_merge_strategylist" ]; then
  >                 echo "$__git_merge_strategylist"
  >                 return
  >         fi
  >         sed -n "/^all_strategies='/{
  >                 s/^all_strategies='//
  >                 s/'//
  >                 p
  >                 q
  >                 }" "$(git --exec-path)/git-merge"
  > }
  > 
  > It takes several seconds to finish that function.

  Maybe something like this?

 Documentation/git-merge.txt            |    4 ++++
 builtin-merge.c                        |    7 +++++++
 contrib/completion/git-completion.bash |    7 +------
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 17a15ac..f3fe1c9 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 'git merge' [-n] [--stat] [--no-commit] [--squash] [-s <strategy>]...
 	[-m <msg>] <remote> <remote>...
 'git merge' <msg> HEAD <remote>...
+'git merge' --show-strategies
 
 DESCRIPTION
 -----------
@@ -37,6 +38,9 @@ include::merge-options.txt[]
 	least one <remote>.  Specifying more than one <remote>
 	obviously means you are trying an Octopus.
 
+--show-strategies::
+	Show all available strategies. For internal use only.
+
 include::merge-strategies.txt[]
 
 
diff --git a/builtin-merge.c b/builtin-merge.c
index de025ac..613c96a 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -802,6 +802,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	const char *best_strategy = NULL, *wt_strategy = NULL;
 	struct commit_list **remotes = &remoteheads;
 
+	/* needed for git bash completion and similar tools */
+	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return 0;
+	}
+
 	setup_work_tree();
 	if (unmerged_cache())
 		die("You are in the middle of a conflicted merge.");
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 158b912..1eea49a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -271,12 +271,7 @@ __git_merge_strategies ()
 		echo "$__git_merge_strategylist"
 		return
 	fi
-	sed -n "/^all_strategies='/{
-		s/^all_strategies='//
-		s/'//
-		p
-		q
-		}" "$(git --exec-path)/git-merge"
+	$(git --exec-path)/git-merge --show-strategies
 }
 __git_merge_strategylist=
 __git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"
-- 
1.6.0.96.g2fad1.dirty

Re: [PATCH] bash-completion: fix getting strategy list

[Matthieu Moy]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Bash completion needs to know what strategies git supports. Maybe
> other similar tools have the same demand. So add 
> "git merge--show-strategies"
            ^^

Silly detail, but a space is missing here.

-- 
Matthieu

Re: [PATCH] bash-completion: fix getting strategy list

[Nguyen Thai Ngoc Duy]

On 8/19/08, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>  > Bash completion needs to know what strategies git supports. Maybe
>  > other similar tools have the same demand. So add
>  > "git merge--show-strategies"
>
>             ^^
>
>  Silly detail, but a space is missing here.

Yes, my poor eyes. Junio if you accept the patch, please add one more
space. Thanks :)

-- 
Duy

Re: [PATCH] bash-completion: fix getting strategy list

[Shawn O. Pearce]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>   > __git_merge_strategies ()
>   > {
>   >         if [ -n "$__git_merge_strategylist" ]; then
>   >                 echo "$__git_merge_strategylist"
>   >                 return
>   >         fi
>   >         sed -n "/^all_strategies='/{
>   >                 s/^all_strategies='//
>   >                 s/'//
>   >                 p
>   >                 q
>   >                 }" "$(git --exec-path)/git-merge"
>   > }
>   > 
>   > It takes several seconds to finish that function.

Youch.
 
>   Maybe something like this?

Yea, this looks reasonable.  ACK from me, though the bigger change
is to builtin-merge and not the completion. Miklos?  Dscho?

> diff --git a/builtin-merge.c b/builtin-merge.c
> index de025ac..613c96a 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -802,6 +802,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  	const char *best_strategy = NULL, *wt_strategy = NULL;
>  	struct commit_list **remotes = &remoteheads;
>  
> +	/* needed for git bash completion and similar tools */
> +	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
> +		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
> +			printf("%s\n", all_strategy[i].name);
> +		return 0;
> +	}
> +
>  	setup_work_tree();
>  	if (unmerged_cache())
>  		die("You are in the middle of a conflicted merge.");
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 158b912..1eea49a 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -271,12 +271,7 @@ __git_merge_strategies ()
>  		echo "$__git_merge_strategylist"
>  		return
>  	fi
> -	sed -n "/^all_strategies='/{
> -		s/^all_strategies='//
> -		s/'//
> -		p
> -		q
> -		}" "$(git --exec-path)/git-merge"
> +	$(git --exec-path)/git-merge --show-strategies
>  }
>  __git_merge_strategylist=
>  __git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"

-- 
Shawn.

Re: [PATCH] bash-completion: fix getting strategy list

[Johannes Sixt]

Nguyen Thai Ngoc Duy schrieb:
> +--show-strategies::
> +	Show all available strategies. For internal use only.
> +

IMO, you don't need to declare this option as internal; offering it for
the public is fine...

> +	/* needed for git bash completion and similar tools */

... which would make this comment slightly odd.

> +	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
> +		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
> +			printf("%s\n", all_strategy[i].name);
> +		return 0;

Improved error checking, but quick and dirty:

+	if (!strcmp(argv[1], "--show-strategies")) {
+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return argc == 2 ? 0 :
+			 error("--show-strategies does not take "
+				"any arguments");

> +	$(git --exec-path)/git-merge --show-strategies

+	git merge --show-strategies

-- Hannes

Re: [PATCH] bash-completion: fix getting strategy list

[Johannes Sixt]

Johannes Sixt schrieb:
> Nguyen Thai Ngoc Duy schrieb:
>> +	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
>> +		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
>> +			printf("%s\n", all_strategy[i].name);
>> +		return 0;
> 
> Improved error checking, but quick and dirty:
> 
> +	if (!strcmp(argv[1], "--show-strategies")) {

Oops, not really improved. This still needs to check for argc >= 2.

> +		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
> +			printf("%s\n", all_strategy[i].name);
> +		return argc == 2 ? 0 :
> +			 error("--show-strategies does not take "
> +				"any arguments");

-- Hannes

Re: [PATCH] bash-completion: fix getting strategy list

[Nguyen Thai Ngoc Duy]

On 8/19/08, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nguyen Thai Ngoc Duy schrieb:
>
> > +--show-strategies::
>  > +     Show all available strategies. For internal use only.
>  > +
>
>
> IMO, you don't need to declare this option as internal; offering it for
>  the public is fine...

On second thought, I don't think the patch's worth it. The code in
git-completion.bash is a hack and I replace it with another the hack.
It won't work for custom merges and git-completion.bash will need to
be synced manually anyway, so maybe this patch will do better:

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 158b912..2fed6ac 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -267,19 +267,8 @@ __git_remotes ()

 __git_merge_strategies ()
 {
-	if [ -n "$__git_merge_strategylist" ]; then
-		echo "$__git_merge_strategylist"
-		return
-	fi
-	sed -n "/^all_strategies='/{
-		s/^all_strategies='//
-		s/'//
-		p
-		q
-		}" "$(git --exec-path)/git-merge"
-}
-__git_merge_strategylist=
-__git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"
+	echo recursive octopus resolve ours subtree
+}

 __git_complete_file ()
 {

-- 
Duy

Re: [PATCH] bash-completion: fix getting strategy list

[Junio C Hamano]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> On second thought, I don't think the patch's worth it. The code in
> git-completion.bash is a hack and I replace it with another the hack.
> It won't work for custom merges and git-completion.bash will need to
> be synced manually anyway, so maybe this patch will do better:

This would be the right thing to do for the 'maint' track.  In the longer
term, especially if we are to actually make the "custom" thing official, I
think teaching "git-merge" to report what are available is the only
sensible approach, though.

Re: [PATCH] bash-completion: fix getting strategy list

[Junio C Hamano]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> On 8/19/08, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Nguyen Thai Ngoc Duy schrieb:
>>
>> > +--show-strategies::
>>  > +     Show all available strategies. For internal use only.
>>  > +
>>
>>
>> IMO, you don't need to declare this option as internal; offering it for
>>  the public is fine...
>
> On second thought, I don't think the patch's worth it.

How about doing this instead?

-- >8 --
completion: find out supported merge strategies correctly

"git-merge" is a binary executable these days, and looking for assignment
to $all_strategies variable does not work well.

When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
commands respectively say:

    $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
    available strategies are: recur recursive octopus resolve stupid ours subtree 
    $ $HOME/git-snap-v1.6.0/bin/git merge -s help
    Could not find merge strategy 'help'.
    Available strategies are: recursive octopus resolve ours subtree.

both on its standard error stream.  We can use this to learn what
strategies are supported.

The sed script is written in such a way that it catches both old and new
message styles ("Available" vs "available", and the full stop at the end).
It also allows future versions of "git merge" to line-wrap the list of
strategies, and add extra comments, like this:

    $ $HOME/git-snap-v1.6.1/bin/git merge -s help
    Could not find merge strategy 'help'.
    Available strategies are: blame recursive octopus resolve ours
    subtree.
    Also you have custom strategies: theirs

    Make sure you spell strategy names correctly.

---
diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
index 158b912..a310040 100755
--- c/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash
@@ -271,15 +271,17 @@ __git_merge_strategies ()
 		echo "$__git_merge_strategylist"
 		return
 	fi
-	sed -n "/^all_strategies='/{
-		s/^all_strategies='//
-		s/'//
+	git merge -s help 2>&1 |
+	sed -n -e '/[Aa]vailable strategies are: /,/^$/{
+		s/\.$//
+		s/.*://
+		s/^[ 	]*//
+		s/[ 	]*$//
 		p
-		q
-		}" "$(git --exec-path)/git-merge"
+	}'
 }
 __git_merge_strategylist=
-__git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"
+__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)
 
 __git_complete_file ()
 {

Re: [PATCH] bash-completion: fix getting strategy list

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

On Wed, Aug 20, 2008 at 02:13:42PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> completion: find out supported merge strategies correctly
> 
> "git-merge" is a binary executable these days, and looking for assignment
> to $all_strategies variable does not work well.
> 
> When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
> commands respectively say:
> 
>     $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
>     available strategies are: recur recursive octopus resolve stupid ours subtree 
>     $ $HOME/git-snap-v1.6.0/bin/git merge -s help
>     Could not find merge strategy 'help'.
>     Available strategies are: recursive octopus resolve ours subtree.
> 
> both on its standard error stream.  We can use this to learn what
> strategies are supported.
> 
> The sed script is written in such a way that it catches both old and new
> message styles ("Available" vs "available", and the full stop at the end).
> It also allows future versions of "git merge" to line-wrap the list of
> strategies, and add extra comments, like this:
> 
>     $ $HOME/git-snap-v1.6.1/bin/git merge -s help
>     Could not find merge strategy 'help'.
>     Available strategies are: blame recursive octopus resolve ours
>     subtree.
>     Also you have custom strategies: theirs
> 
>     Make sure you spell strategy names correctly.

I like this, not adding something to builtin-merge when it's already
there. :)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] bash-completion: fix getting strategy list

[Nguyen Thai Ngoc Duy]

On 8/21/08, Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Wed, Aug 20, 2008 at 02:13:42PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
>  > completion: find out supported merge strategies correctly
>  >
>  > "git-merge" is a binary executable these days, and looking for assignment
>  > to $all_strategies variable does not work well.
>  >
>  > When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
>  > commands respectively say:
>  >
>  >     $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
>  >     available strategies are: recur recursive octopus resolve stupid ours subtree
>  >     $ $HOME/git-snap-v1.6.0/bin/git merge -s help
>  >     Could not find merge strategy 'help'.
>  >     Available strategies are: recursive octopus resolve ours subtree.
>  >
>  > both on its standard error stream.  We can use this to learn what
>  > strategies are supported.
>  >
>  > The sed script is written in such a way that it catches both old and new
>  > message styles ("Available" vs "available", and the full stop at the end).
>  > It also allows future versions of "git merge" to line-wrap the list of
>  > strategies, and add extra comments, like this:
>  >
>  >     $ $HOME/git-snap-v1.6.1/bin/git merge -s help
>  >     Could not find merge strategy 'help'.
>  >     Available strategies are: blame recursive octopus resolve ours
>  >     subtree.
>  >     Also you have custom strategies: theirs
>  >
>  >     Make sure you spell strategy names correctly.
>
>
> I like this, not adding something to builtin-merge when it's already
>  there. :)

Me too. Did not realize it's already there :P

-- 
Duy

Re: [PATCH] bash-completion: fix getting strategy list

[Mikael Magnusson]

2008/8/20 Junio C Hamano <gitster@pobox.com>:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>
>> On 8/19/08, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Nguyen Thai Ngoc Duy schrieb:
>>>
>>> > +--show-strategies::
>>>  > +     Show all available strategies. For internal use only.
>>>  > +
>>>
>>>
>>> IMO, you don't need to declare this option as internal; offering it for
>>>  the public is fine...
>>
>> On second thought, I don't think the patch's worth it.
>
> How about doing this instead?
>
> -- >8 --
> completion: find out supported merge strategies correctly
>
> "git-merge" is a binary executable these days, and looking for assignment
> to $all_strategies variable does not work well.
>
> When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
> commands respectively say:
>
>    $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
>    available strategies are: recur recursive octopus resolve stupid ours subtree
>    $ $HOME/git-snap-v1.6.0/bin/git merge -s help
>    Could not find merge strategy 'help'.
>    Available strategies are: recursive octopus resolve ours subtree.
>
> both on its standard error stream.  We can use this to learn what
> strategies are supported.
>
> The sed script is written in such a way that it catches both old and new
> message styles ("Available" vs "available", and the full stop at the end).
> It also allows future versions of "git merge" to line-wrap the list of
> strategies, and add extra comments, like this:
>
>    $ $HOME/git-snap-v1.6.1/bin/git merge -s help
>    Could not find merge strategy 'help'.
>    Available strategies are: blame recursive octopus resolve ours
>    subtree.
>    Also you have custom strategies: theirs
>
>    Make sure you spell strategy names correctly.
>
> ---
> diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
> index 158b912..a310040 100755
> --- c/contrib/completion/git-completion.bash
> +++ w/contrib/completion/git-completion.bash
> @@ -271,15 +271,17 @@ __git_merge_strategies ()
>                echo "$__git_merge_strategylist"
>                return
>        fi
> -       sed -n "/^all_strategies='/{
> -               s/^all_strategies='//
> -               s/'//
> +       git merge -s help 2>&1 |
> +       sed -n -e '/[Aa]vailable strategies are: /,/^$/{
> +               s/\.$//
> +               s/.*://
> +               s/^[    ]*//
> +               s/[     ]*$//
>                p
> -               q
> -               }" "$(git --exec-path)/git-merge"
> +       }'
>  }
>  __git_merge_strategylist=
> -__git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"
> +__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)
>
>  __git_complete_file ()
>  {

I don't know if i somehow have some weird patch merged in, i tried to check
and didn't find anything, at least. But when i run git merge -s help i get:

Could not find merge strategy 'help'.

available strategies in '/usr/libexec/git-core'
-----------------------------------------------
  file      octopus   ours      recursive resolve   subtree

which i suspect does not work with your regex.

-- 
Mikael Magnusson

Re: [PATCH] bash-completion: fix getting strategy list

[Junio C Hamano]

"Mikael Magnusson" <mikachu@gmail.com> writes:

> I don't know if i somehow have some weird patch merged in, i tried to check
> and didn't find anything, at least. But when i run git merge -s help i get:

Yes, there are some weird patch in 'next'.  Miklos, could you fix the
output format of that thing?

Re: [PATCH] bash-completion: fix getting strategy list

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Mikael Magnusson" <mikachu@gmail.com> writes:
>
>> I don't know if i somehow have some weird patch merged in, i tried to check
>> and didn't find anything, at least. But when i run git merge -s help i get:
>
> Yes, there are some weird patch in 'next'.  Miklos, could you fix the
> output format of that thing?

I do not think the code in 'next' that reuses the "help" thing to show
only the list to stdout while still giving the error message to stderr
makes any sense.

Let's do this.

---

 builtin-merge.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git c/builtin-merge.c w/builtin-merge.c
index 1f9389b..3e8db0d 100644
--- c/builtin-merge.c
+++ w/builtin-merge.c
@@ -110,9 +110,17 @@ static struct strategy *get_strategy(const char *name)
 		}
 	}
 	if (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {
-
-		fprintf(stderr, "Could not find merge strategy '%s'.\n\n", name);
-		list_commands("strategies", longest, &main_cmds, &other_cmds);
+		fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
+		fprintf(stderr, "Available strategies are:");
+		for (i = 0; i < main_cmds.cnt; i++)
+			fprintf(stderr, " %s", main_cmds.names[i]->name);
+		fprintf(stderr, ".\n");
+		if (other_cmds.cnt) {
+			fprintf(stderr, "Available custom strategies are:");
+			for (i = 0; i < other_cmds.cnt; i++)
+				fprintf(stderr, " %s", other_cmds.names[i]->name);
+			fprintf(stderr, ".\n");
+		}
 		exit(1);
 	}
 

Re: [PATCH] bash-completion: fix getting strategy list

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 1697 bytes --]

On Wed, Aug 20, 2008 at 10:07:55PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > Yes, there are some weird patch in 'next'.  Miklos, could you fix the
> > output format of that thing?

You were faster. ;-)

> I do not think the code in 'next' that reuses the "help" thing to show
> only the list to stdout while still giving the error message to stderr
> makes any sense.
> 
> Let's do this.
> 
> ---
> 
>  builtin-merge.c |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git c/builtin-merge.c w/builtin-merge.c
> index 1f9389b..3e8db0d 100644
> --- c/builtin-merge.c
> +++ w/builtin-merge.c
> @@ -110,9 +110,17 @@ static struct strategy *get_strategy(const char *name)
>  		}
>  	}
>  	if (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {
> -
> -		fprintf(stderr, "Could not find merge strategy '%s'.\n\n", name);
> -		list_commands("strategies", longest, &main_cmds, &other_cmds);
> +		fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
> +		fprintf(stderr, "Available strategies are:");
> +		for (i = 0; i < main_cmds.cnt; i++)
> +			fprintf(stderr, " %s", main_cmds.names[i]->name);
> +		fprintf(stderr, ".\n");
> +		if (other_cmds.cnt) {
> +			fprintf(stderr, "Available custom strategies are:");
> +			for (i = 0; i < other_cmds.cnt; i++)
> +				fprintf(stderr, " %s", other_cmds.names[i]->name);
> +			fprintf(stderr, ".\n");
> +		}
>  		exit(1);
>  	}

Given that we don't have 100+ merge strategies like we have 100+ git
commands, I think printing them in one line should not be problematic,
and this definitely improves script usability, so I like it, thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]