Bug 311695

Comment 1 Duncan Keysell 2010-05-05 06:26:38 EDT

Created attachment 167103 [details]
screenshot

Comment 2 Navid Mehregani 2010-08-18 10:46:46 EDT

It's not just package artifacts.  Any artifact that starts with a lowercase letter will generate a warning in the refactor wizard and have the finish button disabled.

As part of fixing this defect, please check to see which artifacts can have a name that starts with a lowercase letter.  If it should be disallowed, then the refactor wizard should generate an ERROR as opposed to a WARNING when disabling the finish button.  If it is supposed to be just a warning, then the finish button should not be disabled.

Comment 3 Daniel Johnson 2010-08-19 19:22:57 EDT

Created attachment 177057 [details]
patch_311695.txt

Patch to allow artifact names to start with a lowercase letter. Will still warn that it is not the recommended naming convention, but it will allow you to commit the change. Currently the comparison to see if a classname already exists is case-insensitive, meaning that you cannot have a Test.java and a test.java. This necessarily means that you cannot rename a class from Test.java to test.java through the Refactor->Rename wizard, since it will think the class already exists. However, you can rename it like this using the Properties view. In fact, the properties view will let you rename an artifact to the same name as one that already exists, and then will not let you change it back once you do, but I think that is a separate bug.

Class Changed: RenameModelArtifactWizardPage.java

Comment 4 Navid Mehregani 2010-08-20 09:41:32 EDT

(In reply to comment #3)
> Created an attachment (id=177057) [details]
> patch_311695.txt
> 
> This necessarily means that you cannot rename a class from Test.java
> to test.java through the Refactor->Rename wizard, since it will think the class
> already exists. However, you can rename it like this using the Properties view.
> In fact, the properties view will let you rename an artifact to the same name
> as one that already exists, and then will not let you change it back once you
> do, but I think that is a separate bug.
> 
> Class Changed: RenameModelArtifactWizardPage.java


Dan, this has been addressed in bug#321023

Comment 5 Navid Mehregani 2010-08-24 14:52:13 EDT

Thanks Dan!  I just reviewed the patch.  Your changes are correct to enable the 'finish' button for a warning.  However, for some of these artifacts we shouldn't be warning the user when the name starts with a lowercase letter.  For example, it's completely valid for package names to start with a lowercase letter.  You should also check to see which other artifact names can start with a lowercase letter and avoid giving the warning message when those artifacts are selected.

I won't commit the attached patch until you have a chance to make these changes.

Comment 6 Daniel Johnson 2010-08-24 20:14:43 EDT

Created attachment 177379 [details]
patch_311695.txt

Will now correctly warn of casing for artifacts both in the create new artifact wizard, and the rename wizard.

Classes Changed:
NewPatternBasedArtifactWizardPage.java
RenameModelArtifactWizardPage.java

Comment 7 Daniel Johnson 2010-08-24 20:16:32 EDT

(In reply to comment #6)
> Created an attachment (id=177379) [details]
> patch_311695.txt
> 
> Will now correctly warn of casing for artifacts both in the create new artifact
> wizard, and the rename wizard.
> 
> Classes Changed:
> NewPatternBasedArtifactWizardPage.java
> RenameModelArtifactWizardPage.java

I am not sure if the solution in the NewPatternBasedArtifactWizardPage is the best, but I could not think of a better way.

Comment 8 Navid Mehregani 2010-08-25 11:28:31 EDT

Reviewed and submitted patch.  I found another issue where the user is allowed to create duplicate packages in the same source folder. 

When creating a new package, user can remove the text under 'artifact package' field, specify something under 'name' field that already exists in the source folder, click on finish!  However, this is a separate issue.