modelsbuilder / ModelsBuilder.Original

The Community Models Builder for Umbraco
MIT License
114 stars 49 forks source link

Feature Request - generate static constants of property names #157

Closed JohnBergman closed 5 years ago

JohnBergman commented 7 years ago

I work a lot with the IContent models, I would like to see something like this added to move the string constants out of my code and into something that can be generated.

For example, in the Image document type, the following is currently generated

    ///<summary>
    /// Height
    ///</summary>
    [ImplementPropertyType("umbracoHeight")]
    public string umbracoHeight
    {
        get { return this.GetPropertyValue<string>("umbracoHeight"); }
    }

I propose that this be added, and used inside the objects appropriately.

public const string umbracoHeight="umbracoheight";

zpqrtbnk commented 7 years ago

Totally makes sense. Idea we have is to have more "meta" data about models such as these constants, a way to get all content types, their model types, their properties, aliases, etc. Won't do immediately but keeping the issue open - definitively on the radar.

JohnBergman commented 7 years ago

Does this change only need to be in TextBuilder? Seems pretty straightforward... perhaps I'll have a go at it and do a pull request.

zpqrtbnk commented 7 years ago

Yes, the other "builders" were a nice idea, and eventually we'll allow ppl to write their own builders, but for the time being, there's only TextBuilder really. Quite happy if you want to have a go at it. We discussed it a while ago with few ppl and I have this notes file here that contains:

// modelsbuilder meta:

var modelTypes = Models.ContentTypes();
var contentType = Models.ContentType<Seo>();
  Models.ContentType(typeof(Seo));
  Models.For.Seo.ContentType;
var contentTypeAlias = Models.Alias<Seo>();
  Models.For.Seo.Alias;
var propertyTypes = Models.PropertyTypes<Seo>()
  Models.PropertyTypes(typeof(Seo));
var propertyType = Models.PropertyType<Seo>(x => x.Keywords);
  Models.For.Seo.For.Keywords.PropertyType;
var propertyAlias = Models.Alias<Seo>(x => x.Keywords);
  Models.For.Seo.For.Keywords.Alias;

var fields = "nodeName, " + Models.Alias<TypeMenu>(x => x.MenuTitle) + ", ...";

var propertyAlias = Models.Seo.Keywords.Alias; // conflict on Seo.
var propreryAlias = Models.Types.Seo.Properties.Keywords.Alias
Models.Alias(x => x.Seo.Alias)
Models.TypeAlias<Seo>()
Models.PropAlias<Seo>().
Models.T.Seo.P.Keywords.Alias

Models.For.Seo.For.Keywords.Alias  

Just copy/pasted without re-reading, this is in no way "the right way to do it", but IIRC the idea was to have some sort of meta, static class that would provide all sorts of infos about models. Syntax was highly work-in-progress, we'd want it to be as friendly as possible, I guess.

Feel free to ping me if you want to discuss it further!

JohnBergman commented 7 years ago

Are you thinking we should add a second nested static class that contains all of the metadata constants, etc, or add all of the metadata as constants inside the currently generated class?

zpqrtbnk commented 7 years ago

Not 100% sure but I am afraid that adding more and more meta-data, models get bloated. So I'd really like considering moving all the meta-data stuff (including what's already generated in model classes) to some other static class eg the Models class above. Or, at least, yes, to a nested class.

The problem we have with adding stuff into models is collisions. If there's a nested class named Meta, then we cannot have a model property named Meta too. Reason why I'd love to move as much as we can outside of models.

Making sense?

tompipe commented 7 years ago

Just discussed this with Stephan at UK Fest Hackathon. I added PR #122 almost a year ago to do just this. However, the above approach makes much more sense. Happy to chip in on the most appropriate way to move forward on this issue.

pmgower commented 6 years ago

@zpqrtbnk any update on this feature request?

I like the idea of having the metadata structured so that it doesn't conflict with potential properties on the model classes.

I'd enjoy the opportunity to help implement this feature although it looks like @tompipe and @JohnBergman have already volunteered as well.

Either way, I think this would be a much-needed addition to the ModelsBuilder because I constantly have to add a Constants static class to store the Aliases of the properties of the document types I end up needing to create using IContent interfaces. Having these generated and available would be a great step in the right direction.

zpqrtbnk commented 6 years ago

Honest update: after a looong time of it being low priority, Umbraco v8 development is resuming and getting traction = basically using all my time at the moment and I cannot find enough time per day to work on this feature. That being said, it's part of some general improvements I want to see in MB.

zpqrtbnk commented 5 years ago

Closing this issue, gathering the metadata discussion in #215