commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benoit Callebaut <benoit.calleb...@euphonynet.be>
Subject RE: [PATCH][Jelly] new features : properties & VFS integration
Date Fri, 23 Dec 2005 22:48:23 GMT
Hello James,
Thanks for highlighting some mistakes.

project.xml has a new dependency on vfs.

I am actually rewritting the whole stuff to make the commons-vfs
integration patch independant of the "property" modification.
I am also writing a lot of test.
Some of them will add extra dependencies mainly on the commons-net
project.
I have also to setup my linux-box at home to support all the services
requested for the test.

On that point, I have a remark more for the VFS guys. In the VFS
properties file, there are hard-coded things like IP address and test
account. It is not portable across systems.

Le dim 18/12/2005 à 23:40, James.Ring@ga.gov.au a écrit :
> Hi Benoit,
> 
> I'm new to the commons-dev list, forgive me if my comments are not
> posted to the right place.
> 
> > -----Original Message-----
> > From: Benoit Callebaut [mailto:benoit.callebaut@euphonynet.be] 
> > Sent: Thursday, 15 December 2005 10:18 PM
> > To: commons-dev@jakarta.apache.org
> > Subject: [PATCH][Jelly] new features : properties & VFS integration
> > 
> > 
> > Hello,
> > This patch add 2 features :
> 
> > Index: src/java/org/apache/commons/jelly/tags/core/FileTag.java
> > [ ... snip ... ]
> > @@ -55,8 +72,42 @@
> >     public void doTag(final XMLOutput output) throws JellyTagException {
> >         try {
> >             if ( name != null ) {
> > +                OutputStream out = null;
> >                 String encoding = (this.encoding != null) ? this.encoding :
> "UTF-8";
> > -                Writer writer = new OutputStreamWriter( new
> FileOutputStream( name, doAppend ), encoding );
> > +                Object obj = null;
> > +                obj = getContext().getProperty("VFSManager");
> > +                if ((obj == null) && (obj instanceof FileSystemManager)){
>                         ^^^^^^^^^^^
> Shouldn't this be obj != null ?
> 
> > +                    manager = (FileSystemManager)obj;
> > +                }
> > +                if (manager == null){
> > +                    log.error("Manager not initialized. Falling back on
> old functionality");
> > +                    if ( name != null ) {
> > +                        out = new FileOutputStream( name, doAppend );
> > +                    }
> > +                }else{
> 
> > Index: project.xml
> 
> Are there actually any changes to project.xml in your patch, or is it all
> formatting? It would be a lot easier
> if superfluous changes were removed from the patch.
> 
> > Index: parent-project.xml
> 
> Same again...
> 
> > Index: src/java/org/apache/commons/jelly/JellyContext.java
> > @@ -262,6 +267,22 @@
> >          return null;
> >      }
> >  
> > +    public Hashtable getProperties(){
> > +        return properties;
> > +    }
> > +    
> > +    public Object setProperty(String name, Object value){
> > +        return properties.put(name,value);
> > +    }
> > +    
> > +    public Object getProperty(String name){
> > +        try{
> > +            return properties.get(name);
> > +        }catch (Exception e){
> > +            return null;
> > +        }
> > +    }
> > +
> 
> I think 
> 
> 	public Object getProperty(String name) {
> 		if (name == null)
> 			return null;
> 		return properties.get(name);
> 	}
> 
> is nicer than catching this exception.
> 
> > Benoit
> > 
> 
> Thanks!
> 
> Regards,
> James
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message