[PATCH] Ant completion improvement

[PATCH] Ant completion improvement

[Jean-Baptiste Quenot]

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

Hello!

Thanks for zsh: it's great.

The example below does not return completion for the "build" target
without the patch (replace ^Is with tabs):

^I^I<target name="build" depends="init">

The example below does not return completion for the "build" target
without the patch:

<target depends="init" name="build">

Please find attached a patch against 4.2.1 to allow ant completion to:

1) match leading tabs before the <target/> elements
2) match the target's "name" attribute not just on the first position

Thanks in advance,
-- 
Jean-Baptiste Quenot
http://caraldi.com/jbq/

[-- Attachment #2: patch-zsh-ant-completion --]
[-- Type: text/plain, Size: 494 bytes --]

--- _ant.orig	Mon Dec  6 11:22:13 2004
+++ _ant	Mon Dec  6 11:40:46 2004
@@ -86,7 +86,7 @@
 	buildfile=${(v)opt_args[(I)(-f|-file|-buildfile)]:-build.xml}
       fi
       if [[ -f $buildfile ]]; then
-        targets=( $(sed -n "s/ *<target name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )
+        targets=( $(sed -n "s/[ 	]*<target *[^>]* name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )
 	_wanted targets expl target compadd -a targets && ret=0
       else
 	_message -e targets target

Re: [PATCH] Ant completion improvement

[Doug Kearns]

On Mon, Dec 06, 2004 at 11:41:19AM +0100, Jean-Baptiste Quenot wrote:

<snip>

> --- _ant.orig	Mon Dec  6 11:22:13 2004
> +++ _ant	Mon Dec  6 11:40:46 2004
> @@ -86,7 +86,7 @@
>  	buildfile=${(v)opt_args[(I)(-f|-file|-buildfile)]:-build.xml}
>        fi
>        if [[ -f $buildfile ]]; then
> -        targets=( $(sed -n "s/ *<target name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )
> +        targets=( $(sed -n "s/[ 	]*<target *[^>]* name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )

You could use [:blank:] here      ^^^^^^ 

>  	_wanted targets expl target compadd -a targets && ret=0
>        else
>  	_message -e targets target

Regards,
Doug

Re: [PATCH] Ant completion improvement

[Doug Kearns]

Hello Jean-Baptiste,

On Mon, Dec 06, 2004 at 11:41:19AM +0100, Jean-Baptiste Quenot wrote:
> Hello!
> 
> Thanks for zsh: it's great.
> 
> The example below does not return completion for the "build" target
> without the patch (replace ^Is with tabs):
> 
> ^I^I<target name="build" depends="init">
> 
> The example below does not return completion for the "build" target
> without the patch:
> 
> <target depends="init" name="build">
> 
> Please find attached a patch against 4.2.1 to allow ant completion to:

Thanks. I was just about to commit this, but I'm not sure it's necessary.
 
> 1) match leading tabs before the <target/> elements

See below.

> 2) match the target's "name" attribute not just on the first position

It looks as though a variant of this has already been committed as a
result of your patch posted to the list on 24/08/04.
 
> Thanks in advance,

> --- _ant.orig	Mon Dec  6 11:22:13 2004
> +++ _ant	Mon Dec  6 11:40:46 2004
> @@ -86,7 +86,7 @@
>  	buildfile=${(v)opt_args[(I)(-f|-file|-buildfile)]:-build.xml}
>        fi
>        if [[ -f $buildfile ]]; then
> -        targets=( $(sed -n "s/ *<target name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )
> +        targets=( $(sed -n "s/[ 	]*<target *[^>]* name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )

Why not just:

           targets=( $(sed -n "s/<target name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )

Either way I don't see why this would fail with leading tabs. What am I
missing?

>  	_wanted targets expl target compadd -a targets && ret=0
>        else
>  	_message -e targets target

Regards,
Doug

Re: [PATCH] Ant completion improvement

[Jean-Baptiste Quenot]

* Doug Kearns:
> > 2) match the target's "name" attribute not just on the first position
> 
> It looks as though a variant of this has already been committed as a
> result of your patch posted to the list on 24/08/04.

OK I understand now, you mean it's committed but not in zsh 4.2.1?
I will try to get the latest zsh from CVS.

> Why not just:
> 
>            targets=( $(sed -n "s/<target name=[\"']\([^\"']*\)[\"'].*/\1/p" < $buildfile) )
> 
> Either way I don't see why this would fail with leading tabs. What am I
> missing?

No it doesn't fail with leading tabs, it's just a « cosmetic »
cleanup.

Sorry for posting this patch a second time, I didn't remember the
patch submitted Tue, 24 Aug 2004 as I use zsh 4.2.1.

Just a note, I would replace:

<target[^>]*

With:

<target *[^>]*

Just to be sure that we don't match <targetfoobar ...

Sorry for the noise,
-- 
Jean-Baptiste Quenot
http://caraldi.com/jbq/

Re: [PATCH] Ant completion improvement

[Doug Kearns]

On Wed, Dec 08, 2004 at 02:33:49PM +0100, Jean-Baptiste Quenot wrote:

<snip> 

> Just a note, I would replace:
> 
> <target[^>]*
> 
> With:
> 
> <target *[^>]*
> 
> Just to be sure that we don't match <targetfoobar ...

Agreed.

Is there any need for us to keep the / *<target/ ?
                                      ^^
> Sorry for the noise,

No noise...

Regards,
Doug

Re: [PATCH] Ant completion improvement

[Jean-Baptiste Quenot]

* Doug Kearns:

> Is there any need for us to keep the / *<target/ ? ^^

Well,  this is  again for  cosmetic  reasons AFAICT  when the  zsh
developer is manually viewing the results of the sed command.  But
this causes  problems when file  has tabs instead of  spaces, then
you don't have completion anymore.   So either remove leading " *"
(preferred) or specify  the character class :blank: to  be sure to
match all blanks.  The XML spec does not require blanks before the
tag.  About anything could appear there.

Best regards,
-- 
Jean-Baptiste Quenot
http://caraldi.com/jbq/

Re: [PATCH] Ant completion improvement

[Doug Kearns]

On Thu, Dec 09, 2004 at 12:57:44AM +1100, Doug Kearns wrote:
> On Wed, Dec 08, 2004 at 02:33:49PM +0100, Jean-Baptiste Quenot wrote:
> 
> <snip> 
> 
> > Just a note, I would replace:
> > 
> > <target[^>]*
> > 
> > With:
> > 
> > <target *[^>]*
> > 
> > Just to be sure that we don't match <targetfoobar ...
> 
> Agreed.

Accept, of course, this _will_ match <targetfoobar ;-)

I'll take a look at coming up with something sufficiently general in the
morning.

Regards,
Doug