ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam Murdoch" <adammurdoch...@yahoo.com>
Subject RE: [WIP]Selectors
Date Mon, 04 Mar 2002 11:42:42 GMT

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:bruce@callenish.com]
> Sent: Monday, 4 March 2002 5:06 PM
> To: ant-dev@jakarta.apache.org
> 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:ajkuiper@wxs.nl">ajkuiper@wxs.nl</a>
>   * @author <a href="mailto:umagesh@rediffmail.com">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:stefan.bodewig@epost.de">Stefan Bodewig</a>
>   * @author <a href="mailto:umagesh@rediffmail.com">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:jon@clearink.com">jon@clearink.com</a>
>   * @author <a href="mailto:stefan.bodewig@epost.de">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:ant-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:ant-dev-help@jakarta.apache.org>


Mime
View raw message