ant-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 52706] [PATCH] Make Execute class extensible to allow custom CommandLauncher implementation
Date Mon, 11 Jun 2012 18:01:07 GMT
https://issues.apache.org/bugzilla/show_bug.cgi?id=52706

--- Comment #5 from Vimil <vimilsaju@yahoo.com> ---
(In reply to comment #3)
> (In reply to comment #2)
> 
> > I think CommandLauncher can be extracted to a new class. The CommandLauncher
> > class accesses 'environmentCaseInSensitive' which is internal to Execute
> > class, but that can be moved to a static constructor block within the
> > Execute class itself.
> 
> Yes, sounds good.

I have extracted CommandLauncher, CommandLauncherProxy, Java13CommandLauncher,
MacCommandLauncher, OS2CommandLauncher, PerlScriptCommandLauncher,
ScriptCommandLauncher, VmsCommandLauncher and WinNTCommandLauncher to new
classes under org.apache.tools.ant.taskdefs package.

I faced a minor issue when extracting MacCommandLauncher, it was referring to
'antWorkingDirectory' String field in Execute class. I replaced that reference
with System.getProperty("user.dir")

> 
> > However the sub-classes of CommandLauncher class i.e
> > PerlScriptCommandLauncher, ScriptCommandLauncher and VmsCommandLauncher
> > access FILE_UTILS singleton which is internal to Execute, So I am wondering
> > whether they should remain within the Execute class itself.
> 
> The FILE_UTILS is just a performance enhancement, it is backed by a
> Singleton instance in FileUtils so it wouldn't cause too much overhead to
> copy it around to each class (or add a protected one in CommandLauncher).
> 
I've added FILE_UTILS as a static protected field in CommandLauncher.

> > I wanted to do a diff, but I couldn't find a suitable tool in windows that
> > generates a diff? Do you know of any tool that would do that?
> 
> Subversion ;-)
> 
> svn diff
> 
> inside a working copy.

I was able to use the svn diff tool :). It seems eclipse has built-in svn
support and can generate patches.  I have attached the patch to this reply :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

Mime
View raw message