Finally coding...

..And done. Blink and you miss it. After all the hunting for what to change and negotiating how to do this, the code for the part we all agreed to came together in a surprisingly short time.

I refactored some of the code in NewTemplateWizardPage. I replaced the code inside the constructTypeStub method, which used to hard code the type header and the brackets. Out of that, I broke out the type header construction part into a new routine I called "constructTypeHeader". I also created a new routine in StubUtility, exposed through CodeGeneration (like all the rest of its classes) called getTypeContent which is patterned after the current getTypeComment, though most of the template reading methods follow the same pattern. Users pass the header data into this routine, and it inserts it into the template. Then the rest of the routines already pass the constructed type content on into the new file template routine, so no need for changes there.

So I ran all the current tests, and the part that took the longest was debugging and getting the current tests to run. If you add a code template into Eclipse, and you add a new variable into the text of the default template without putting in all the supporting code for the variable in the Eclipse classes, Eclipse will internally throw and log an error, and blank out your whole code template as invalid. To me, since I wasn't checking the logs when I ran it, it looked like my new template dissapeared. I thought I had erased one of my changes to implement the original template in one of my manual sync operations described below. I checked the code over and over and could not find what I had left out. After a few traces of the initialization code, I finally found the error. When I changed the ${type_header} text in the code template to "type_header" so the system wouldn't try to process it as a template variable, the template re-appeared. After that, it took only a few minutes to actually implement the code for the ${type_header} variable.

Once I started running the tests, all the current tests in NewTypeWizardTest failed because of the default for my "New Java types" typecontent code template didn't match the original behavior. The problem was in the blank lines, and after some tracing (since I couldn't seem to interpret the output of the test failure correctly) I found the formula was two newlines between the class braces, and one newline after the last class brace. Then it passed all the tests. I ran an "AllAllTests" the way Dirk told me to, and it passed all 3179 tests.

I made patches in Eclipse's CVS view for my changes to jdt.ui and jdt.ui.tests, and sent it in.

- - - - - - - - - -
Here is a patch that adds a "typecontent" template that the system uses to
process user-definable content that will be substituted for the
${type_definition} variable in the "newtype" code template. This patch also
adds a ${type_header} variable for users to insert into the "typecontent"
template.
The default text for the new template matches the original version, and so
passes all current tests with one modification which adds the new template
variable so the test variable counts match what is now in the template store.
No new automated tests are added to insure different user edits of the new
template behave as expected (though I tested some manually). Let me know if
you have any changes in the above patch you want or any other work you want me
to do on this.
Thanks,
- John
- - - - - - - - - - - - -

All in all, it was not as bad as I thought it would be as far as design. Since we simplified the problem substantially by not allowing users to direct where Eclipse inserts the default method stubs, I didn't have to change any of the current APIs, and could simply add one that fit in the style of the others.

There are plenty of things in this code that are not how I would have done it. I still don't like the mixing of model and view in the NewTypeWizardPage, and the splitting of responsibilities so that some code is generated in NewTypeWizardPage, and some is generated in StubUtility. I still believe this code is more difficult than it should be for current clients to use, and it is difficult to test. Like many software developers, I have strong, individualistic opinions about the way things should be. But, it was easy enough to make this change keeping in the style of the current code, and that is part of the deal, especially for us non-committers. This was a good exercise for me as I am used to having more control over the details of how my code is written, and fitting into someone else's style was interesting.

I think we will have to confront some of these issues more directly if we decide to allow the users to direct where the system inserts new method stubs, but that is a more complex question for another day.

One more comment on this exercise, I never did find a good solution for syncing the Eclipse source tree and keeping my changes up with the new integration releases from Eclipse. I tried several things and none of them worked, and I ended up saving off my changes, pulling down new builds from Eclipse, and merging my changes back in using file merge utilities. I did this twice, and it was an awkward pain in the butt. There must be a formula for syncing to Eclipse integration labels, and I'm going to ask the experts and report back.