Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Support MessageBundles as properties #651

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

EotT123
Copy link
Contributor

@EotT123 EotT123 commented Dec 27, 2024

Add support for using MessageBundles as properties.

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 28, 2024

I've used reflection to modify the values of properties when changes occur, but it would be more elegant if all fields could simply delegate the calls to the appropriate resource bundle, eliminating the need for reflection. Unfortunately, this isn't feasible because the field values are computed directly. However, it could be possible by using a getter method in conjunction with Manifold Properties to simulate direct field access. For now, though, this approach is beyond my current understanding of the code.

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 28, 2024

After going through and analyzing the code, I might be able to do it after all. I'm currently working on it. To be continued.

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 28, 2024

I managed to make it work.
The expression String test = messages.foo.bar.test; is now compiled to String test = messages.foo.bar.getTest();.

By calling getTest(), I can return the value from the current resource bundle without relying on the field, which would need to be overridden (via reflection) every time the locale changes.

public String getTest() {
    return messages._resourceBundle.getString("foo.bar.test");
}

This approach improves performance and, in my opinion, makes the (generated) code cleaner.

For compiling String test = messages.foo.bar.test; to String test = messages.foo.bar.getTest();, I reused parts of the manifold-props module,

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 28, 2024

Another use case that could be supported:

Consider an enum that holds localized messages:

public enum MyEnum {
    VALUE_FOO(Message.value.foo),
    VALUE_BAR(Message.value.bar);

    private String value;

    MyEnum(String value) {
        this.value = value;
    }

    // getter + toString
}
...
System.out.println(MyEnum.VALUE_FOO);

This works fine until the Locale of the MessageBundle changes. The value isn't re-evaluated, which can be problematic.

I already added support for full properties as strings by using the getValueByName method, e.g.:

public enum MyEnum {
    VALUE_FOO("value.foo"),
    VALUE_BAR("value.bar");

    private String value;

    MyEnum(String value) {
        this.value = value;
    }

    // getter + toString
}
...
System.out.println(Message.getValueByName(MyEnum.VALUE_FOO.getValue()));

This approach resolves the problem, but it comes at the cost of defeating the initial goal of not using string values (which might contain typos) and losing other advantages such as compile-time checks and content assistance.

To address this, we could introduce lazy properties. A lazy property is an object whose toString method directly retrieves the value from the message bundle when it's accessed. This ensures that the value is evaluated only when needed, and it will always return the correct value depending on the current locale of the Message.

Here's how it might look:

public enum MyEnum {
    VALUE_FOO(Message.lazy().value.foo),
    VALUE_BAR(Message.lazy().value.bar);

    private LazyMessage value;

    MyEnum(LazyMessage value) {
        this.value = value;
    }

    // getter + toString
}
...
System.out.println(MyEnum.VALUE_FOO);

What do you think? Implementing this shouldn't be too difficult, as it would be quite similar to the existing code. I can take a look at it tomorrow.

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 28, 2024

One more thing: IntelliJ no longer marks the properties in the .properties file as used. This can probably be addressed in the manifold-ij plugin, but I have no idea how to do that.

@rsmckinney
Copy link
Member

Re laziness, you could maybe simplify this using a lambda:

VALUE_FOO(()-> Message.value.foo)
. . .
MyEnum(Supplier<String> value) {
    this.value = value;
}

Shrug.

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 28, 2024

That's indeed a simple solution. I was overthinking it due to all the new possibilities that Manifold offers and hadn't considered this straightforward approach.

@EotT123
Copy link
Contributor Author

EotT123 commented Dec 30, 2024

This PR doesn't work when a project is imported into another project, as the field accessor to getter replacement is not performed in that context. The previous approach (using reflection to update the fields) works, but it's quite inelegant.

@EotT123 EotT123 changed the title Support MessageBundles as properties WIP: Support MessageBundles as properties Dec 30, 2024
{
SrcClass propertyValueClass = new SrcClass(PROPERTY_VALUE_CLASS_NAME, srcClass, Kind.Class )
.modifiers( Modifier.PUBLIC | Modifier.STATIC | Modifier.FINAL )
.addInterface( "CharSequence" )
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, extending String isn't possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants