Community
Participate
Working Groups
For some projects, like GNU Classpath, it would be very convenient if the reformatting code in the JDT core were available as a standalone application or an ant task. This would let us standardize on Eclipse to enforce our code formatting style, while placating those developers who don't use Eclipse.
You would at least need a Eclipse headless when running your ant task. The API of the code formatter are using Eclipse text API. So Eclipse is needed. You would also need a way to get the code formatter preferences. This requires at least the JDT/Core plugin.
Running eclipse headless would be acceptable for my purposes. As far as preferences go, ideally I would be able to point the formatter at an XML file describing the desired coding style. That way I could set it up once in Eclipse, export it, check it in to Classpath, and all Classpath developers (including those not using Eclipse for development) could easily use it.
Created attachment 17416 [details] adds an application extension to org.eclipse.jdt.core for command line formatting The only thing I don't like about this patch is that it hard codes a value for the config version. The value should come from ProfileVersioner.CURRENT_VERSION, however this class cannot be included for two reasons: it is internal to org.eclipse.jdt.ui and org.eclipse.jdt.core does not require org.eclipse.jdt.ui (nor should it). Is there interest in refactoring these two plugins to make ProfileVersioner an external class of org.eclipse.jdt.core? Comments are appreciated.
Created attachment 17493 [details] updated patch to fix a small typo in the above patch
Created attachment 18373 [details] adds a main method so that it can be used as a stand-alone app I made a stand-alone app out of the patch and I'm calling it EFJ (Eclipse Formatter for Java). To see how I did it, download the tarball and read the ant build file: http://www.bagu.org/eclipse/efj-3.0.1.tar.bz2
Deferring post 3.1
Comment on attachment 18373 [details] adds a main method so that it can be used as a stand-alone app This doesn't work with anything >= 3.1M5. I'll have to make an rcp app.
Just to clarify the comment above, the patch that I made obsolete today was a version of my original commandline formatting patch with an additional a main method. I did this to satisfy some requests to have an Eclipse based commandline formatter without having to install the all of Eclipse platform, JDT and the supporting GUI libraries (gtk+ or whatever). The use case for this comes from the GNU Classpath developers. They would like to put this minimal program (which I call EFJ ) on a server without having to use up a tonne of space and without having to install a GUI toolkit. I think their goal is to run a nightly cron job which checks out the code, formats is and checks it back in. This would also allow EFJ to be used in a server side commit hook. The patch that I made obsolete was for ECJ. It worked in 3.0.1, but when I tried it with 3.1M5 it didn't work becuase jdt.core does not get automatically loaded with >= 3.1M5 but for some reason it does with 3.0.1. This means that EFJ cannot access the default configuration for the code formatter with 3.1M5 and it produces a NPE. So the patch that I would like to have accepted does not have this GUI-less requirement, this is something that I'm going to do on my own. (I think I'm going to try to do it as an GUI-less RCP app if this is possible - I still need to investigate this). The patch that is not obsolete is for Eclipse proper. There is still one problem with the patch - it requires a constant value from jdt.ui (see FIXME comment in the patch for details). I think that the best way around this is to move the configuration code to jdt.core and make a public API so that jdt.ui can get access to it. I think that this patch is very useful and I am willing to do the refactoring work when you are ready to put this into Eclipse. It is for this reason that I respectfully request that you to change this bug from “RESOLVED LATER” to “NEW - Target Milestone ...”. FWIW, Red Hat is picking up this patch in our natively compiled version of Eclipse (RPM >= 3.1.0_fc-0.M5.18). Thanks, Ben
The third paragragh should say EFJ not ECJ :)
I would like to add that being able to run the formatter from the command line (even requiring the GUI plugins) would be great. The current alternative is to use Jalopy for command line formatting and force all developers to install the Jalopy Eclipse plugin to acheive the exact same formatting through the IDE.
Any plans on getting this into 3.1?
It would be nice to see this patch get into the jdt. Let me know if there is anything you'd like me to change in the patch before you apply it. Thanks, Ben
Created attachment 33430 [details] workspace patch for jdt.core and jdt.ui Patch ChangeLog =============== * Move ProfileVersion Numbers from jdt.ui to jdt.core so that the formatter application can use them. * Update patch for HEAD
Created attachment 33432 [details] workspace patch for jdt.core and jdt.ui fix small typo in patch
Reopen.
Martin, Would it be possible for you to release the jdt/ui part of the patch?
I'm not sure if the patch is a good idea. You are reusing the file that JDT.UI uses to share formatter profiles. Currently the file format of a profile is an implementation detail of jdt.ui, under the full control on jdt.ui, including version numbers, how if is loaded and stored. To share this in two plugins will complicate our life. The profile version numbers that you move down have no meaning in jdt core. The correct way would be to move down the full profile manager, including profile updating on new versions down. Or we use the new plugin jdt.core.manipulation for this (it's a head-less plugin intended to host jdt.core funtionality)
Martin, I don't see a problem for moving down the version manager to the core level instead of the UI. If it can go inside the core.manipulation plugin, why not? Let me know if you want to do that since it has to be done before M5.
I'm afaid I won't have time for that until M5, and this will require some refactoring and several new API's to be defined. The export profile file format is also not 100% the right choice for this problem, as it can contain more than one profile in it. What about another way of passing the compiler options? E.g. as properties file containing the key/value pairs?
Hi guys, Thanks for considering this patch. I actually tried to move the version manager code to the jdt.core, but i found that I ended up with a requirement for ui.workbench which i didn't think the jdt.core plugin would want. Let me know if I was wrong though. Martin: I didn't realise that export profile file format could have more than one profile in it. The reason I like it is because it's easy for users to generate. I think the best way to go is to have the application default to the first profile, but allow users to choose a different profile with a command line flag. Thoughts? Anyway, I would like to do the work for any changes that need to be made, but I would need a little direction as I don't know the jdt.ui plugin well. Thanks, Ben
I think Martin's suggestion to use a properties file is a good one. This application doesn't need anything else than the formatter options. A simple format as a properties file seems to be good enough. The concern is this case is more to be able to create this properties file. If the user is using project's specific options, then the file org.eclipse.jdt.core.prefs located in the .settings folder follows this format. It is a .prefs file, but it has the same format. So a simple copy/paste in a properties file would make the trick. Otherwise, we can create a small action on a Java project that would export such a properties file based on the project preferences.
Created attachment 33760 [details] patch against jdt.core and jdt.doc.user I moved to using the org.eclipse.jdt.core.prefs file for the config format. I think it should be ok for users to just copy out this file if they want custom formatting options. I've also added some documentation. Let me know if there is anything that needs to be done. Thanks, Ben
Hi Ben, I just wanted to point out that some of the paths in your last patch are a bit messed up. You have some relative paths from the root of the source directory to things in 'plugins/org.eclipse.jdt.doc.user', but you have other things hardcoded under '/home/eclipse'. I had to go through them and change them to not be hardcoded to apply cleanly.
Created attachment 33870 [details] Patch with manually cleaned up paths Obviously this won't help it if your patch changes in the future, but this can be cleanly :)
Igor, the patch that I attached is in the Eclipse Workspace Patch format and as such it won't apply with the command line patch utility. I just tested it with Eclipse and it applied fine. If you want to generate patches for that utility, you should use a recent I-build of eclipse to the apply the patch and then make two separate patches. The manual cleanup you did may break the Eclipse Workspace Patch format.
Ben, Several notes regarding your last patch. I would slightly modify the way the command line is processed. I don't like too much the creation of an ArrayList instance simply to iterate over it. I would also change the processing to return null as soon as the command line is invalid or the file list is empty. This simplifies the test on the file list in the run method. No need to check for isEmpty(). I wonder if the test: if (file.getPath().endsWith(".java")) ... should not be replaced with: if (Util.isJavaLikeFileName(file.getPath()) Another thing. I am surprised that there is a test for ".java" file in the formatDirTree, but not for formatFile. I would also ensure that the file is "formattable". You create one formatter per file. It might be a good idea to reuse the same one. We might also convert the messages to the new format using fields and not a ResourceBundle. I can take care of those changes. Simply let me know what you think.
(In reply to comment #26) > I would slightly modify the way the command line is processed. > I don't like too much the creation of an ArrayList instance simply to iterate > over it. Yeah, I believe I did that so that I could easily remove arguments from the ArrayList once they are found. I'm ok with changing it though. > I would also change the processing to return null as soon as the command line > is invalid or the file list is empty. This simplifies the test on the file > list in the run method. No need to check for isEmpty(). Sounds good. > I wonder if the test: > if (file.getPath().endsWith(".java")) ... > should not be replaced with: > if (Util.isJavaLikeFileName(file.getPath()) Yeah, that would be good - I wasn't aware of that method. > Another thing. I am surprised that there is a test for ".java" file in the > formatDirTree, but not for formatFile. I would also ensure that the file is > "formattable". That's an oversight on my part. Thanks for catching this. > You create one formatter per file. It might be a good idea to reuse the same > one. Good point, I agree. > We might also convert the messages to the new format using fields and not a > ResourceBundle. Oh right, the format of messages changed since I originally wrote the patch. It would good to use the new style. > I can take care of those changes. Simply let me know what you think. Great. Thanks again for reviewing the patch and for implementing the changes. Ben
I'll try to come up with a new patch soon that would include all changes in comment 26.
Ok. if you don't have time I can make the patch. Just let me know.
No, thank you. I'll do it tomorrow. I'd like it to be released for M5.
Fixed and released in HEAD. Ben, Could you please review my changes? They reflect what is described in comment 26. Let me know if you have any concern.
Created attachment 34106 [details] small comment clean up Hi Oliver, I looked over your changes and they look good. This small patch cleans up the displayHelp() comment. Thanks again for reviewing the patch and making these changes.
No problem. Released your last patch.
Verified for 3.2 M5 using build I20060214-0010. We definitely have a batch formatter here. I believe that the tuning should go in new bugs, since the function is already sound. As far as tuning is concerned, may consider picking up reasonable defaults for the configuration, or else require the configuration file explicitly on the command line (-config suggests an option; if it is mandatory, it is no more an option). I also got slightly different results formatting directly from Eclipse vs using the new application, but this may trace back to a difference in prefs I was unaware of. Note that none of these preclude the use of the delivered function as a predictable batch formatter.
*** Bug 61100 has been marked as a duplicate of this bug. ***
Maxime, Have you filed any bugs regarding the formatter application. If so, can you list the bug numbers and/or add me as a cc? Thanks, Ben
