http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java File dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java (right):
http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode80 dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:80: public static boolean isValidModuleName(String moduleName) { On 2011/06/29 18:56:00, jbrosenberg wrote:
Will this cause previously building modules to now have errors?
It was a part of the original top level module check, I'm just expressing it in a different way to make sure we catch ".." on the end of the string. It's an odd enough case that I don't think it will cause a problem. http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode87 dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:87: for (int i = 0; i < parts.length - 1; i++) { On 2011/06/29 18:56:00, jbrosenberg wrote:
Should we issues a warning about a bad identifier on the last part of
the module
name? Maybe enforce it in the future? Or not?
There is a prudent reason to make the package java idents - they correspond to Java package names. But the last part of the module name doesn't have a Java analog. Since we didn't enforce it before, and there is no functional reason to require it to be a Java identifier, I'm not sure its worth the disruption. http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java File dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java (left): http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java#oldcode310 dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java:310: throws UnableToCompleteException { On 2011/06/29 18:56:00, jbrosenberg wrote:
so, this validation is needed only on nested modules, but not
top-level ones? The top level module gets fed into nestedLoad() (that's why the Invalid..Foo.gwt.xml throws an exception. http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java File dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java (right): http://gwt-code-reviews.appspot.com/1467810/diff/1/dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java#newcode66 dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java:66: "com.google.gwt.dev.cfg.testdata.naming.NestedInvalid", false); On 2011/06/29 18:56:00, jbrosenberg wrote:
Maybe add a comment here, since this identifier looks ok, but the
error is that
this module inherits from a malformed module identifier (was confusing
to me at
first).
Done. http://gwt-code-reviews.appspot.com/1467810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
