Hi, Some comments. I think the selector idea is fantastic, so these comments are just implementation nit-picks.
ISelector: * Can we do something about the name? Almost none of the interfaces in Ant use the IBlah naming convention. Can we change it to FileSelector, or something like that? * assignProject() isn't really necessary. If a selector needs to get hold of the project, it can extend ProjectComponent, and the introspector will take care handing it a project. No point setting up parallel infrastructure for doing this. * Maybe add "throws BuildException" to isSelected(). How about we lose checkForError() as well, and get isSelected() to assert that things are good, throwing a BuildException if they're not. * Given the above, it might be worth making the interface completely stateless, and pass the base directory in to isSelected(), rather than using assignBasedir(). SelectorContainer: * Should assignProject() and assignBasedir() be calling through to the nested selectors? * The createBlah() methods should probably be addBlah() methods instead. NotSelector: * isSelected() probably should assert that there is exactly one nested selector. Otherwise you've got a nor selector, not a not selector. FilenameSelector: * Do we really need 'negate'? How about we take it out and wait to see if it's useful. * The guts of this selector look suspiciously like the guts of DirectoryScanner. Can you refactor the matching code into a ScannerUtil class that both FileNameSelector and DirectoryScanner share? (it's mostly all static, shouldn't be a drama). IExtendSelector: * Hmmm. setAttrib9()? Can we use the same pattern for extensibility that the file filter code uses? Whether it's a good or bad pattern, we need consistency. I suspect that there's a good candidate somewhere there for a base class gets used by the mapper, file filter, and selector extensibility beans, to take care of 90% of it. MajoritySelector: * Nice idea. PatternSet: I think you're overloading a little by adding the nested selectors to PatternSet. Let's leave PatternSet as a set of patterns, and add a new selector container data-type (like AndSelector, say). Get FileSet to extend that, rather than PatternSet. We probably still want PatternSet to *be* a selector, so rather than have PatternSet extend AndSelector, we should have it implement ISelector, as a specialisation of FileNameSelector. FileSet will need to deal with PatternSet specially, to remain backwards compatible (since the PatternSets are aggregated before they are evaluated). Adam > -----Original Message----- > From: Bruce Atherton [mailto:[EMAIL PROTECTED] > Sent: Monday, 4 March 2002 5:06 PM > To: [EMAIL PROTECTED] > Subject: [WIP]Selectors > > > Here is the Selectors as a work in progress, just so everyone can > see how it > is coming along. I still have some known problem areas, and much of this > hasn't been tested, but the earlier the feedback the better, right? > > I've included a test file that downloads ant 1.4.1 and does a few > selections > with it. This needs to be modified for the real testing framework > (which I > haven't paid any attention to so far, I'm afraid, but that I will have to > learn soon I guess) but it gives some reassurances that things > are working as > expected for at least simple situations. > > All changes so far pass the full testing framework "ant test" target, BTW. > > Anyway, the renaming is done, <and>, <or>, and <not> are > available, basically > <extendselect> works, in theory references should work (never > tested, so I'm > not sure), embedded <patternset>s in selector containers have > known problems, > I've simplified maintaining the static selectors by making a slight (but > fully backwards compatible) change to the inheritance of <patternset> and > <fileset>, and I've implemented a couple more selectors: > <majority>, which is > there to show people they can do more with selector containers than just > boolean; and <filenameselect>, which is also available as an > <extendselect> > class for testing purposes and will eventually play a role in fixing > <patternset>s that are within an explicit selector container. > > I've included the patch for the 3 changed files again, for > convenience. The > code changes are minimal so far, really, the biggest impact is to the > comments. > > All comments, criticisms, complaints, and any other c-words > welcome. Thanks. > > > Index: DirectoryScanner.java > =================================================================== > RCS file: > /home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/Director > yScanner.java,v > retrieving revision 1.22 > diff -u -b -r1.22 DirectoryScanner.java > --- DirectoryScanner.java 18 Feb 2002 18:27:58 -0000 1.22 > +++ DirectoryScanner.java 4 Mar 2002 06:39:12 -0000 > @@ -58,25 +58,31 @@ > import java.util.Vector; > import java.util.StringTokenizer; > > +import org.apache.tools.ant.types.selectors.SelectorScanner; > +import org.apache.tools.ant.types.selectors.ISelector; > + > /** > * Class for scanning a directory for files/directories which > match certain > * criteria. > * <p> > - * These criteria consist of a set of include and exclude patterns. With > these > - * patterns, you can select which files you want to have > included, and which > - * files you want to have excluded. > + * These criteria consist of a set of selectors which have been > specified. > + * With these selectors you can select which files you want to > have included. > + * Files which are not selected are excluded. > * <p> > * The idea is simple. A given directory is recursively scanned > for all files > - * and directories. Each file/directory is matched against a set > of include > + * and directories. Each file/directory is matched against a set of > selectors, > + * including special support for matching agains filenames with > include and > * and exclude patterns. Only files/directories which match at least one > - * pattern of the include pattern list, and don't match any > pattern of the > - * exclude pattern list will be placed in the list of files/directories > found. > + * pattern of the include pattern list or other file selector, and don't > match > + * any pattern of the exclude pattern list or fail to match against a > required > + * selector will be placed in the list of files/directories found. > * <p> > * When no list of include patterns is supplied, "**" will be used, which > * means that everything will be matched. When no list of > exclude patterns is > - * supplied, an empty list is used, such that nothing will be excluded. > + * supplied, an empty list is used, such that nothing will be > excluded. When > + * no selectors are supplied, none are applied. > * <p> > - * The pattern matching is done as follows: > + * The filename pattern matching is done as follows: > * The name to be matched is split up in path segments. A path > segment is the > * name of a directory or file, which is bounded by > * <code>File.separator</code> ('/' under UNIX, '\' under Windows). > @@ -140,7 +146,7 @@ > * <a href="mailto:[EMAIL PROTECTED]">[EMAIL PROTECTED]</a> > * @author <a href="mailto:[EMAIL PROTECTED]">Magesh Umasankar</a> > */ > -public class DirectoryScanner implements FileScanner { > +public class DirectoryScanner implements FileScanner, SelectorScanner { > > /** > * Patterns which should be excluded by default. > @@ -170,10 +176,15 @@ > /** The patterns for the files to be excluded. */ > protected String[] excludes; > > - /** The files which matched at least one include and no excludes. */ > + /** Selectors that will filter which files are in our > candidate list. */ > + protected ISelector[] selectors = null; > + > + /** The files which matched at least one include and no excludes > + * and were selected. > + */ > protected Vector filesIncluded; > > - /** The files which did not match any includes. */ > + /** The files which did not match any includes or selectors. */ > protected Vector filesNotIncluded; > > /** > @@ -182,7 +193,9 @@ > */ > protected Vector filesExcluded; > > - /** The directories which matched at least one include and > no excludes. > */ > + /** The directories which matched at least one include and > no excludes > + * and were selected. > + */ > protected Vector dirsIncluded; > > /** The directories which were found and did not match any > includes. */ > @@ -194,6 +207,16 @@ > */ > protected Vector dirsExcluded; > > + /** The files which matched at least one include and no excludes and > + * which a selector discarded. > + */ > + protected Vector filesDeselected; > + > + /** The directories which matched at least one include and > no excludes > + * but which a selector discarded. > + */ > + protected Vector dirsDeselected; > + > /** Whether or not our results were built by a slow scan. */ > protected boolean haveSlowResults = false; > > @@ -711,6 +734,17 @@ > } > } > > + > + /** > + * Sets the selectors that will select the filelist. > + * > + * @param selectors specifies the selectors to be invoked on a scan > + */ > + public void setSelectors(ISelector[] selectors) { > + this.selectors = selectors; > + } > + > + > /** > * Sets the list of exclude patterns to use. All '/' and '\' > characters > * are replaced by <code>File.separatorChar</code>, so the separator > used > @@ -752,7 +786,8 @@ > > /** > * Scans the base directory for files which match at least > one include > - * pattern and don't match any exclude patterns. > + * pattern and don't match any exclude patterns. If there > are selectors, > + * then the files must pass muster there, as well. > * > * @exception IllegalStateException if the base directory was set > * incorrectly (i.e. if it is <code>null</code>, doesn't > exist, > @@ -783,14 +818,20 @@ > filesIncluded = new Vector(); > filesNotIncluded = new Vector(); > filesExcluded = new Vector(); > + filesDeselected = new Vector(); > dirsIncluded = new Vector(); > dirsNotIncluded = new Vector(); > dirsExcluded = new Vector(); > + dirsDeselected = new Vector(); > > if (isIncluded("")) { > if (!isExcluded("")) { > + if (isSelected("",basedir)) { > dirsIncluded.addElement(""); > } else { > + dirsDeselected.addElement(""); > + } > + } else { > dirsExcluded.addElement(""); > } > } else { > @@ -838,8 +879,8 @@ > /** > * Scans the given directory for files and directories. > Found files and > * directories are placed in their respective collections, > based on the > - * matching of includes and excludes. When a directory is > found, it is > - * scanned recursively. > + * matching of includes, excludes, and selectors. When a directory is > + * found, it is scanned recursively. > * > * @param dir The directory to scan. Must not be <code>null</code>. > * @param vpath The path relative to the base directory (needed to > @@ -876,12 +917,21 @@ > if (file.isDirectory()) { > if (isIncluded(name)) { > if (!isExcluded(name)) { > + if (isSelected(name,file)) { > dirsIncluded.addElement(name); > if (fast) { > scandir(file, name+File.separator, fast); > } > } else { > everythingIncluded = false; > + dirsDeselected.addElement(name); > + if (fast && couldHoldIncluded(name)) { > + scandir(file, name+File.separator, fast); > + } > + } > + > + } else { > + everythingIncluded = false; > dirsExcluded.addElement(name); > if (fast && couldHoldIncluded(name)) { > scandir(file, name+File.separator, fast); > @@ -900,9 +950,14 @@ > } else if (file.isFile()) { > if (isIncluded(name)) { > if (!isExcluded(name)) { > + if (isSelected(name,file)) { > filesIncluded.addElement(name); > } else { > everythingIncluded = false; > + filesDeselected.addElement(name); > + } > + } else { > + everythingIncluded = false; > filesExcluded.addElement(name); > } > } else { > @@ -965,6 +1020,24 @@ > } > > /** > + * Tests whether a name should be selected. > + * > + * @param name the name to check for selecting > + * @return <code>false</code> when at least one selector > says that the > + * name should not be selected, <code>true</code> otherwise. > + */ > + protected boolean isSelected(String name, File file) { > + if (selectors != null) { > + for (int i = 0; i < selectors.length; i++) { > + if ((selectors[i].isSelected(name, file)) == false) { > + return false; > + } > + } > + } > + return true; > + } > + > + /** > * Returns the names of the files which matched at least one of the > * include patterns and none of the exclude patterns. > * The names are relative to the base directory. > @@ -1023,6 +1096,25 @@ > } > > /** > + * Returns the names of the files which were selected. The names > + * are relative to the base directory. This involves performing > + * a slow scan if one has not already been completed. > + * > + * @return the names of the files which were selected. > + * > + * @see #slowScan > + */ > + public String[] getDeselectedFiles() { > + slowScan(); > + int count = filesDeselected.size(); > + String[] files = new String[count]; > + for (int i = 0; i < count; i++) { > + files[i] = (String)filesDeselected.elementAt(i); > + } > + return files; > + } > + > + /** > * Returns the names of the directories which matched at > least one of > the > * include patterns and none of the exclude patterns. > * The names are relative to the base directory. > @@ -1076,6 +1168,25 @@ > String[] directories = new String[count]; > for (int i = 0; i < count; i++) { > directories[i] = (String)dirsExcluded.elementAt(i); > + } > + return directories; > + } > + > + /** > + * Returns the names of the directories which were selected. > The names > + * are relative to the base directory. This involves performing a > + * slow scan if one has not already been completed. > + * > + * @return the names of the directories which were selected. > + * > + * @see #slowScan > + */ > + public String[] getDeselectedDirectories() { > + slowScan(); > + int count = dirsDeselected.size(); > + String[] directories = new String[count]; > + for (int i = 0; i < count; i++) { > + directories[i] = (String)dirsDeselected.elementAt(i); > } > return directories; > } > Index: types/FileSet.java > =================================================================== > RCS file: > /home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/types/Fi > leSet.java,v > retrieving revision 1.22 > diff -u -b -r1.22 FileSet.java > --- types/FileSet.java 26 Jan 2002 20:16:22 -0000 1.22 > +++ types/FileSet.java 4 Mar 2002 06:39:13 -0000 > @@ -58,6 +58,9 @@ > import org.apache.tools.ant.FileScanner; > import org.apache.tools.ant.DirectoryScanner; > import org.apache.tools.ant.Project; > +import org.apache.tools.ant.types.selectors.ISelector; > +import org.apache.tools.ant.types.selectors.SelectorScanner; > +import org.apache.tools.ant.types.selectors.AndSelector; > > import java.io.File; > import java.util.Stack; > @@ -74,7 +77,7 @@ > * @author <a href="mailto:[EMAIL PROTECTED]">Stefan Bodewig</a> > * @author <a href="mailto:[EMAIL PROTECTED]">Magesh Umasankar</a> > */ > -public class FileSet extends DataType implements Cloneable { > +public class FileSet extends PatternSet implements Cloneable { > > private PatternSet defaultPatterns = new PatternSet(); > private Vector additionalPatterns = new Vector(); > @@ -301,6 +304,19 @@ > > ds.setIncludes(defaultPatterns.getIncludePatterns(p)); > ds.setExcludes(defaultPatterns.getExcludePatterns(p)); > + if (ds instanceof SelectorScanner) { > + SelectorScanner ss = (SelectorScanner)ds; > + ISelector[] selectors = getSelectors(p); > + for (int i = 0; i < selectors.length; i++) { > + selectors[i].assignBasedir(dir); > + selectors[i].assignProject(p); > + String msg = selectors[i].checkForError(); > + if (msg != null) { > + throw new BuildException(msg); > + } > + } > + ss.setSelectors(selectors); > + } > if (useDefaultExcludes) { > ds.addDefaultExcludes(); > } > Index: types/PatternSet.java > =================================================================== > RCS file: > /home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/types/Pa > tternSet.java,v > retrieving revision 1.19 > diff -u -b -r1.19 PatternSet.java > --- types/PatternSet.java 26 Jan 2002 20:16:22 -0000 1.19 > +++ types/PatternSet.java 4 Mar 2002 06:39:13 -0000 > @@ -58,6 +58,8 @@ > > import org.apache.tools.ant.BuildException; > > +import org.apache.tools.ant.types.selectors.*; > + > import java.io.File; > import java.io.BufferedReader; > import java.io.FileReader; > @@ -79,7 +81,7 @@ > * @author Jon S. Stevens <a > href="mailto:[EMAIL PROTECTED]">[EMAIL PROTECTED]</a> > * @author <a href="mailto:[EMAIL PROTECTED]">Stefan Bodewig</a> > */ > -public class PatternSet extends DataType { > +public class PatternSet extends AndSelector { > private Vector includeList = new Vector(); > private Vector excludeList = new Vector(); > private Vector includesFileList = new Vector(); > @@ -332,6 +334,13 @@ > createExclude().setName(excl[i]); > } > } > + > + ISelector[] selectors = other.getSelectors(p); > + if (selectors != null) { > + for (int i=0; i<selectors.length; i++) { > + appendSelector(selectors[i]); > + } > + } > } > > /** > @@ -347,7 +356,7 @@ > } > > /** > - * Returns the filtered include patterns. > + * Returns the filtered exclude patterns. > */ > public String[] getExcludePatterns(Project p) { > if (isReference()) { > @@ -409,7 +418,7 @@ > } > > /** > - * Read includesfile ot excludesfile if not already done so. > + * Read includesfile and excludesfile contents if not > already done so. > */ > private void readFiles(Project p) { > if (includesFileList.size() > 0) { > @@ -449,10 +458,13 @@ > } > } > > - public String toString() > - { > - return "patternSet{ includes: " + includeList + > - " excludes: " + excludeList + " }"; > + public String toString() { > + StringBuffer buf = new StringBuffer("patternSet{ includes: " + > + includeList + " excludes: " + excludeList); > + buf.append(super.toString()); > + buf.append(" }"); > + > + return buf.toString(); > } > > } > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
