Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add TypeDescriptionProviders to XLinq #33082

Merged
merged 5 commits into from
Oct 29, 2018
Merged

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Oct 26, 2018

This adds TypeDescriptionProviders to XAttribute and XElement

To do this without introducing a TypeConverter dependency in XML I had to push
TypeDescriptionProviderAttribute down.

This still causes TypeConverter to gain a dependency on XML, but right now that's the direction
we've been heading with TypeConverter.

I had to use the string overload of TypeDescriptionProviderAttribute in order to have a soft
dependency on TypeConverter.

Fixes #32641

@ericstj ericstj requested review from krwq and weshaggard October 26, 2018 23:50
This adds TypeDescriptionProviders to XAttribute and XElement

To do this without introducing a TypeConverter dependency in XML I had to push
TypeDescriptionProviderAttribute down.

This still causes TypeConverter to gain a dependency on XML, but right now that's the direction
we've been heading with TypeConverter.

I had to use the string overload of TypeDescriptionProviderAttribute in order to have a soft
dependency on TypeConverter.
@ericstj
Copy link
Member Author

ericstj commented Oct 27, 2018

test NETFX x86 Release Build


namespace MS.Internal.Xml.Linq.ComponentModel
{
class XTypeDescriptionProvider<T> : TypeDescriptionProvider
Copy link
Member

Choose a reason for hiding this comment

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

Nit: missing explicit visiblity here and elsewhere, e.g. internal

@@ -0,0 +1,576 @@
// Licensed to the .NET Foundation under one or more agreements.
Copy link
Member

Choose a reason for hiding this comment

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

I assume all of this is just ported effectively as-is from netfx?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as is: I kept the dotnet-bot commit to illustrate that. I'll run the codeformatter to fix up the style issues. I only ran IDE formatting which missed naming/visibility modifiers.

class XElementDescendantsPropertyDescriptor : XPropertyDescriptor<XElement, IEnumerable<XElement>>
{
XDeferredAxis<XElement> value;
XName changeState;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: missing _ prefix on fields

@ericstj ericstj self-assigned this Oct 29, 2018
@ericstj ericstj added this to the 3.0 milestone Oct 29, 2018
I decided to remove this from the reference assemblies.  None  of the usages I found would require
presense in the reference assemblies and it would expose too much internal surface which we might
want to change in the future (type name, and assembly).
@@ -211,6 +212,7 @@ public partial class XDocumentType : System.Xml.Linq.XNode
public override System.Threading.Tasks.Task WriteToAsync(System.Xml.XmlWriter writer, System.Threading.CancellationToken cancellationToken) { throw null; }
}
[System.Xml.Serialization.XmlSchemaProvider(null, IsAny=true)]
[System.ComponentModel.TypeDescriptionProvider("MS.Internal.Xml.Linq.ComponentModel.XTypeDescriptionProvider`1[[System.Xml.Linq.XElement, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]],System.ComponentModel.TypeConverter")]
Copy link
Member

@krwq krwq Oct 29, 2018

Choose a reason for hiding this comment

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

on github it looks like there is something wrong with indentation in here (this whole file and others) - please make sure this is correct

Copy link
Member Author

Choose a reason for hiding this comment

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

This file is no longer part of this commit. I decided not to expose this in the reference assembly.

Copy link
Member

@krwq krwq left a comment

Choose a reason for hiding this comment

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

I'm lacking context on why this is needed but I'm fine with the changes. Please double check the indentation before the attributes as it looks slightly wrong on github

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

Looks reasonable

@ericstj ericstj merged commit a01c44c into dotnet:master Oct 29, 2018
@ViktorHofer
Copy link
Member

@ericstj src/MS/Internal/Xml/Linq/ComponentModel/XComponentModel.cs is dead code. Any reason to keep it?

@ericstj
Copy link
Member Author

ericstj commented Feb 7, 2019

It is not dead code at all. It's used by the following attributes:

[System.ComponentModel.TypeDescriptionProvider("MS.Internal.Xml.Linq.ComponentModel.XTypeDescriptionProvider`1[[System.Xml.Linq.XAttribute, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]],System.ComponentModel.TypeConverter")]

[System.ComponentModel.TypeDescriptionProvider("MS.Internal.Xml.Linq.ComponentModel.XTypeDescriptionProvider`1[[System.Xml.Linq.XElement, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]],System.ComponentModel.TypeConverter")]

The typedescriptor tech resolves this dependency at runtime. Have a look at the tests to see how its used.

@ViktorHofer
Copy link
Member

I didn't notice that, thanks.

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Bring over XComponentModel code from desktop

* Format code and add License.

* Add TypeDescriptionProviders to XLinq

This adds TypeDescriptionProviders to XAttribute and XElement

To do this without introducing a TypeConverter dependency in XML I had to push
TypeDescriptionProviderAttribute down.

This still causes TypeConverter to gain a dependency on XML, but right now that's the direction
we've been heading with TypeConverter.

I had to use the string overload of TypeDescriptionProviderAttribute in order to have a soft
dependency on TypeConverter.

* Format XComponentModel source for access / naming

* Remove TypeDescriptionProvider from reference assemblies

I decided to remove this from the reference assemblies.  None  of the usages I found would require
presense in the reference assemblies and it would expose too much internal surface which we might
want to change in the future (type name, and assembly).


Commit migrated from dotnet/corefx@a01c44c
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants