Bug 192750 - Compile-time bundle keys
Compile-time bundle keys
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: -- Other --
7.0
All All
: P2 with 1 vote (vote)
: 7.0
Assigned To: Jesse Glick
issues@platform
: API, API_REVIEW_FAST
Depends on: 196789 209020 194958 196104
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-01 15:37 UTC by Jesse Glick
Modified: 2012-02-29 22:21 UTC (History)
2 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Proposed patch, minus apichanges (22.99 KB, patch)
2010-12-01 18:04 UTC, Jesse Glick
Details | Diff
Example usage in hudson module (3.88 KB, patch)
2010-12-01 18:17 UTC, Jesse Glick
Details | Diff
Revised patch generating Bundle.properties and accepting non-identifier keys (23.51 KB, patch)
2010-12-03 11:34 UTC, Jesse Glick
Details | Diff
New patch to hudson module (11.72 KB, patch)
2010-12-03 11:36 UTC, Jesse Glick
Details | Diff
New patch (40.63 KB, patch)
2010-12-03 15:09 UTC, Jesse Glick
Details | Diff
Revised demo patch (11.44 KB, patch)
2010-12-03 15:11 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2010-12-01 15:37:11 UTC
Inspired by: http://netbeans.dzone.com/nbbundle-with-annotations

Find a way to manage NbBundle messages that is less cumbersome and error-prone that manually editing Bundle.properties and looking up keys.
Comment 1 Jesse Glick 2010-12-01 18:04:48 UTC
Created attachment 103506 [details]
Proposed patch, minus apichanges
Comment 2 Jesse Glick 2010-12-01 18:17:23 UTC
Created attachment 103507 [details]
Example usage in hudson module
Comment 3 Jesse Glick 2010-12-01 18:45:41 UTC
Please review this proposal. I think an inspection of the sample usage patch from comment #2 shows that the resulting code is much easier to read; and localizable keys used in a method are defined right there, making it simple to move code around, whereas currently a large part of refactoring NB modules consists of hunting down bundle keys and moving them by hand.


Some things that are missing:

1. apichanges.xml, @since, spec version, the usual.

2. Support in the form module for adding @Keys on initComponents and using that for localizable labels. (Would supersede the special move refactoring hook in the form module.)

3. A hint in the Java editor to replace an existing call to NbBundle.getMessage with @Keys (where available), stripping off any unnecessary qualifier prefix and converting the key to a Java identifier. The effect would be similar to the sample usage patch.


I don't have any measurements of the performance impact of converting large amounts of code to use @Keys; clearly Bundle.properties gets split into smaller files, which could be good or bad (depending on how many classes from a package get loaded in a typical session); clearly new helper classes are introduced, though these are rather small when compiled. I rejected the idea of inlining English keys into the helper bundle class since translators, apisupport branding, etc. rely on English keys being present in *.properties files.

It might be possible to consolidate all helper classes and all properties files in a given package into one, if this seems necessary for performance. The processor then gets considerably more complicated by supporting incremental compilation of subsets of the source tree (need to read and rewrite Bundle.java). It might also be necessary in that case to pick a different name than "Bundle.properties", to prevent conflicts with legacy bundle files during incremental conversion of a module to @Keys. Another downside is that you are forced to either prepend an ugly prefix like "DefiningClassName_" to every key and helper method name, or require the user to manually avoid conflicts between compilation units in the same package (as they do today, though with compile-time verification).


The workflow could be friendlier, but I can't think of anything that can get around the fundamental restriction that an annotation processor cannot modify existing classes, nor inspect details of sources. E.g. it would be nice if you could just write

@Localizable
private static String bad_file(File f) {
    return "No such file: " + f;
}

and have this generate

bad_file=No such file: {0}

in some properties file, and rewrite the method to call NbBundle. But this is not possible, at least not without calling into javac implementation classes the way e.g. Lombok does.
Comment 4 krissco 2010-12-01 19:16:00 UTC
This made me think of the Extended Java Editor plugin.

Here's a couple screenshots of what that plugin accomplishes:
http://misundberg.blogspot.com/2010/03/im-using-netbeans-6.html

I didn't find out much info on the plugin - it is available in the standard update center.
Comment 5 Jesse Glick 2010-12-01 19:25:33 UTC
Yes, that plugin makes it easier to read sources using NbBundle in the IDE. It does not make it easier to write them, or to refactor them, or to read version control diffs, or to use other editors, etc.; whereas @Keys helps with all these things (but you need to change your code to use it).
Comment 6 Jesse Glick 2010-12-03 10:15:20 UTC
Realized that it is probably necessary to generate a single Bundle.properties per package, just to avoid "churn" in localizations. For the same reason, necessary to allow arbitrary keys in the annotation; can generate a valid Java identifier during processing.

Annotation should probably be called @Messages rather than @Keys. Seems to be OK to call the helper class Bundle[.java]; even if someone calls ResourceBundle.getBundle (not NbBundle!), since the helper class is not assignable to ResourceBundle it will be ignored and Bundle.properties will be used.

Generated code can probably be simplified to use NbBundle.getMessage.
Comment 7 Jesse Glick 2010-12-03 11:34:14 UTC
Created attachment 103570 [details]
Revised patch generating Bundle.properties and accepting non-identifier keys

Does not yet properly handle incremental compilation, so after changing one source file in a package and building, it is possible for there to be errors (fixed by a clean build or passing all sources in the package to javac).
Comment 8 Jesse Glick 2010-12-03 11:36:12 UTC
Created attachment 103571 [details]
New patch to hudson module

Code is valid, but form editor will not yet display message value in preview.
Comment 9 Jesse Glick 2010-12-03 15:09:18 UTC
Created attachment 103584 [details]
New patch

API changes. Integration with LayerBuilder. Ability to keep a source Bundle.properties which will be appended to. Support for incremental compilation. Support for comments on keys. Better formatting of generated Javadoc, including helpful parameter names.
Comment 10 Jesse Glick 2010-12-03 15:11:56 UTC
Created attachment 103585 [details]
Revised demo patch

Shows incremental conversion of module to use @Messages. No longer trying to use on form. Demonstrates displayName="#..." integration (action can now truly be all in one file). Documented message format parameters.
Comment 11 Jesse Glick 2010-12-03 15:16:11 UTC
(In reply to comment #3)
> Some things that are missing:
> 
> A hint in the Java editor to replace an existing call to NbBundle.getMessage

(or "#..." in an annotation)

4. Usage from apisupport templates.
Comment 12 Jesse Glick 2010-12-08 21:42:40 UTC
core-main #2b8342ab1fbd
Comment 13 Quality Engineering 2010-12-10 06:18:55 UTC
Integrated into 'main-golden', will be available in build *201012100001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/2b8342ab1fbd
User: Jesse Glick <jglick@netbeans.org>
Log: #192750: @NbBundle.Messages.
Comment 14 edvin 2010-12-23 13:13:33 UTC
From the JavaDoc, it seems like the comment line will dictate the argument/variable name for the message when using message bundles, ie:

@Messages({
         "# {0} - file path",
         "dialog.message=The file {0} was invalid."
})

results in:

class Bundle {
     static String dialog_message(Object file_path) {...}
}

So the variable name file_path is taken from the comment line, right?

Wouldn't it be much nicer if you wrote:

@Messages({
         "dialog.message=The file {file_path} was invalid."
})

which then generated the same bundle and added the comment line to the Bundle.properties file automatically:

# {0} - file path
message=The file {0} was invalid.

Also, I wonder why "dialog.message" is converted to just "message" in the bundle file? So "window.message" and "dialog.message" would overwrite each other?
Comment 15 Jesse Glick 2011-01-04 15:22:12 UTC
(In reply to comment #14)
> So the variable name file_path is taken from the comment line, right?

Yes.

> Wouldn't it be much nicer if you wrote:
> 
> @Messages({
>          "dialog.message=The file {file_path} was invalid."
> })

Perhaps, but then the property value would not be a legal MessageFormat pattern. It would be tricky for the processor to convert such a value to a message format, especially when you consider choice formats and other esoterica. Also it would be harder for an editor hint to use @Messages (under development) to convert old code.

> I wonder why "dialog.message" is converted to just "message" in the
> bundle file?

It's not, just a mistake in Javadoc which I will fix.
Comment 16 Quality Engineering 2011-01-06 09:15:21 UTC
Integrated into 'main-golden', will be available in build *201101060001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/1b786ffc196b
User: Jesse Glick <jglick@netbeans.org>
Log: #192750 comment #14: Javadoc mistake.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo