Community
Participate
Working Groups
I am trying to externalize some strings in the e4.tools project. The properties files is located in the OSGI-INF folder. I am unable to select this because the wizard requires a source folder. I understand that Messages.java should be in a source folder, but is there a reason that messages.properties needs to be in one? I have patched the JDT classes to allow for this. Before I post the patch to Gerrit, I am asking if anyone feels this is a bad idea.
Created attachment 242794 [details] screenshot: dialog modification When the Project Folder text box is not empty, the Source Folder and Package widgets are disabled. I wanted to keep the original Source Folder and Package widgets, as most users are used to having those filtered dialogs. If anyone has issues with this approach, I would appreciate feedback ASAP before I spend any more time on it. Thanks!
Comment on attachment 242794 [details] screenshot: dialog modification I'm not sure about that solution in terms of usability. Showing three fields where at least one is not necessary doesn't feel right. For a properties file IMHO it doesn't make sense to force a user to select the source folder and the package. I would suggest to only have the folder dialog, as using this you are also able to select a package in the source folder. If the source/package selection need to stay, I would suggest to have some radio button component that enables either the source/package or the project folder input field(s). This would make it clearer to a user what to do.
I am going on the assumption that there was a reason for requiring a source folder and package. Perhaps it was because many users put both files in the same package and it made it easier to find. It will be trivial to remove the two existing field editors. I would vote in favor of this, unless there are any objections.
JDT/Core doesn't implement any wizards :) -> moving. I'm guessing the following motivation: it must be ensured that the properties file is always available on the classpath at runtime. If it's in a source folder this is implied (unless you miss to package your code, too). For other locations we'd need validation that knows about your build configuration, perhaps build.properties or whatever drives your packaging. This may not be feasible for JDT...
It seems to me that the runtime availability of the properties file has nothing to do with the process of externalizing the strings at development time. Isn't that actually the whole point of a string resource file? I can see why the accessor class file must be, as it has runtime dependencies. It seems to me when I was under the hood that it was probably just easier to reuse the same field editors for the accessor class and resource.
While thinking about this ... did you check the Externalize Strings wizard in a plain Java project? I think in the generated Java class the properties file is referenced in code. Therefore it needs to be in the classpath, which might be a reason for the current state of the dialog. That relation needs to be checked to get clear about the dialog.
I'd recommend you wait for a comment from JDT/UI, who know better why things are as they are ...
My solution is working in both bundles and plain java projects. I have tested it using both standard and eclipse style substitutions. I have also tested it with the resource file in a bundle, java project source folder, osgi folder, top level folder, as well as in an separate resource project that is not even a java project. It works in all cases. All the existing underlying implementation really needed was an IContainer and the name of the resource file. The resource file in this refactoring really has nothing to do with Java Projects or OSGI bundles. I also found a possible bug in the current implementation that was causing the value column to not refresh when changing the accessor class combo to an alternate properties file. This was happening when the eclipse mechanism was checked, causing the property values to not load in the table when selecting the property file in the dialog. @Stephan, I left the two existing fields, along with all the underlying code, so things are still 'as they are'. But I added a feature that has zero impact on previous usage, so IMHO things are also 'better then they were'.
https://git.eclipse.org/r/#/c/26196/
We currently don't have time to look at the patch in detail, but I have several concerns: - I doubt it works at runtime without the folder being on the class path - The UI gets more complicated for 99% of the user not needing this feature - The code gets more complicated - This probably also requires work regarding our NLS hover, NLS hyperlinks and find broken externalized strings features I'm leaving the bug open for now. Hopefully we'll have some time during 4.5 to look at the proposed change in detail.
(In reply to Dani Megert from comment #10) > We currently don't have time to look at the patch in detail, but I have > several concerns: > - I doubt it works at runtime without the folder being on the class path The eclipse mechanism does not need the properties file to be on the class path, and it definitely works. > - The UI gets more complicated for 99% of the user not needing this feature I can turn the new field on with a user preference, or only turn it on if the eclipse mechanism is enabled. His will hide it from users. > - This probably also requires work regarding our NLS hover, NLS hyperlinks > and find broken externalized strings features Would be nice, but that is not why I filed this bug or created the patch. The specific issue I had was related to entering strings into the wizard. If this feature is not used or enabled, the code is the same as it was, with the exception of a couple of field refactorings.
(In reply to Steven Spungin from comment #11) > > - This probably also requires work regarding our NLS hover, NLS hyperlinks > > and find broken externalized strings features > > Would be nice, but that is not why I filed this bug or created the patch. > The specific issue I had was related to entering strings into the wizard. Not, not just nice. If users use this new feature, then existing tools need to work as well.
(In reply to Dani Megert from comment #12) > (In reply to Steven Spungin from comment #11) > > > - This probably also requires work regarding our NLS hover, NLS hyperlinks > > > and find broken externalized strings features > > > > Would be nice, but that is not why I filed this bug or created the patch. > > The specific issue I had was related to entering strings into the wizard. > > Not, not just nice. If users use this new feature, then existing tools need > to work as well. Just because the java file may not be able to get the property values at runtime for these other features, it should not stop the refactoring from being able to load/store values (and for that matter this should not even be limited to a file (e.g. database) The only issue I ran into was this: when opening the Externalize Wizard, I could not calculate the properties file based of the the cu sent in. This required me to manually reselect a saved mapping from the combo, or create a new one. A small price to pay.
