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

Provide option for generated code to use Language Fallback #235

Open
jvtroyen opened this issue Apr 16, 2020 · 7 comments
Open

Provide option for generated code to use Language Fallback #235

jvtroyen opened this issue Apr 16, 2020 · 7 comments

Comments

@jvtroyen
Copy link

jvtroyen commented Apr 16, 2020

Currently, if you want to provide language fallback on properties, you need to provide partial classes and implement it for each Variant property.

It would be nice if there was an appSetting to enable generated code to do this automatically.

[ImplementPropertyType("message")]
public string Message => this.Value("message", fallback: Fallback.ToLanguage);

(I've already started work on a PR #236)

@Shazwazza
Copy link
Collaborator

IIRC there is a plan within MB (and also within Embedded MB) to generate extension methods for each property. This already occurs for things like Children and Children() and it is valid to have an extension method named the same as a property. The reason why this idea was chosen was to make everything consistent and also to provide the ability to get values by culture since currently this requires an ugly this.Value(x => x.Message, "en-AU") type of syntax, instead it would be this.Message("en-AU"). But these ext methods should include other functionality like fallbacks. Then there is no need for appsettings, etc... it's just adding more extension method being generated.

we came up with this concept with @zpqrtbnk a while back but didn't get around to it. I still think this is a good approach and makes everything consistent.

thoughts?

@jvtroyen
Copy link
Author

I created a PR, but it generated a new issue (#236). Sorry, I'm new to the whole PR thing and may have missed how to do the correct flow.

My first thought in regards to your feedback: the extension methods are good in itself, as they open up many possibilities, but developers would still have to "do something" for each property, which is exactly what I'm trying to avoid here. Instead of using this.Message, it would require using this.Message(Fallback.ToLanguage), no?

I would like other developers and those who only make views backed by models developed by others to not have to worry about this, nor have to know which properties are Variant and which are not.

If the generated code is Variant-aware, the properties have fallback incorporated and you are back to simply using this.Message.

@Shazwazza
Copy link
Collaborator

Ah i see what you are saying. All good, just not sure if that work will overlap with the ext stuff or not, maybe not!

@jvtroyen
Copy link
Author

I think both approaches could live side-by-side, as it's only a change to the existing property-getter, not interfering or inhibiting any extension-methods, as you call them.

Thanks for your thoughts.

Is there a way to link my PR to this issue or is it too late now?

@Shazwazza
Copy link
Collaborator

@jvtroyen it's basically linked with the mention of it, else you can edit the very first comment in this thread with a link to it too

@ronaldbarendse
Copy link

Like I've commented on PR #236 (comment), if this gets implemented, it should be done as an attribute (similair to RenamePropertyType).

@ronaldbarendse
Copy link

One thing to note though: Fallback is a struct, so it can't be added as default parameter value. This might make adding the default fallback a little tricky (especially if you want to combine it with other named parameters, without getting unambiguous method declarations). This also means the fallback isn't exposed in the API/IntelliSense, so it needs to be documented correctly on the generated property.

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

No branches or pull requests

3 participants