-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
XML comments on partial methods aren't merged #60730
Comments
Without any spec to support my intuition, I would have expected only the docs from the declaration to count, but I guess merging them would be okay too. |
Thanks, @jcouv. In my case, the motivation is a source generator being able to augment any comments the user may have written. |
@RikkiGibson, do we expect this to be fixed for C# 11 / .NET 7? |
This current behavior is per dotnet/csharplang#5193. @RikkiGibson Let me know what the updated rules should be (especially for when a partial definition exists without an implementation part). I'm interested in taking a look at |
It would also be awesome to see this for type declarations. Our project extensively uses source generators to add partial definitions to over 90% of classes, and some of those generators would benefit greatly from being able to augment the existing XML comments on the class itself. |
I don't have a lot of bandwidth to look at this in the immediate term. But I'd be happy to review any proposal for how augmentation of XML comments would work, which takes into account:
If somebody decides to write such a proposal, it would be good to post it as a discussion in csharplang. |
Any update on this? I see that the last comment is from a year ago |
There is no update on this |
That's sad - but partially understandable |
EDIT: Please vote thumbs up if you agree - or thumbs down if you don't but also comment (with a quote to mine) why you don't like it I would make a proposal, maybe for the time being implement it the same way as done with CLASS comments, so concatenate them - maybe preferably based on alphabetical file name order or something. You could go a step further and merge the Currently the following two partial classes: /// MyLibary.cs
/// <summary>
/// MyLibrary class.
/// </summary>
public partial class MyLibrary
{
/// <summary>
/// Add two integers.
/// </summary>
/// <param name="a"></param>
/// <param name="b"></param>
/// <returns xmlns:x="@" x:type.clarion="real" x:type.javascript="int64">integer</returns>
public static partial int Add(int a, int b);
} // MyLibrary.part.cs
/// <generation>
/// <source>MyNative/External.cs</source>
/// <generation>1</generation>
/// <hash>1</hash>
/// <date>2021-10-01</date>
/// <author>ArjSoftware</author>
/// </generation>
public partial class MyLibrary
{
/// <generation>
/// <source>MyNative/External.cs</source>
/// <generation>1</generation>
/// <hash>1</hash>
/// <date>2021-10-01</date>
/// </generation>
public static partial int Add(int a, int b)
{
return a + b;
}
} Result in the following XML documentation: <?xml version="1.0"?>
<doc>
<assembly>
<name>MyNative</name>
</assembly>
<members>
<member name="T:Test.MyLibrary">
<summary>
MyLibrary class.
</summary>
<generation>
<source>MyNative/External.cs</source>
<generation>1</generation>
<hash>1</hash>
<date>2021-10-01</date>
<author>ArjSoftware</author>
</generation>
</member>
<member name="M:Test.MyLibrary.Add(System.Int32,System.Int32)">
<generation>
<source>MyNative/External.cs</source>
<generation>1</generation>
<hash>1</hash>
<date>2021-10-01</date>
</generation>
</member>
</members>
</doc> As you can see - the XML doc for the partial method is overridden - the So my proposal - just concatenate it as is already done for the CLASS - maybe order them in a logical order of Some tags/references to hopefully speed a discussion about this up: |
Love this idea. I wanted to add xmldocs to public methods with their impl (which is mostly configuration) // source
public partial void ConfigureLoggingDefaults()
{
services.AddLogging(options => { ... })
} // generated
/// <summary>
/// <code>
/// services.AddLogging(options => { ... })
/// </code>
/// </summary>
public partial void ConfigureLoggingDefaults(); This doesn't need "merging" xml comments as long as the source doesn't have any comments of its own. |
@alrz |
Hi, sorry I missed this at the time. At a glance simply concat'ing the XML nodes as we already do with types seems reasonable to me. I think certain warnings on redundant doc elements would have to be adjusted to make reasonable usages valid. But that's OK. What I would like to know more about, though, is, how various doc tools in the ecosystem today react to methods and properties with such XML. e.g. are they handling concatenation internally? Are they dropping some of the parts? If so, which parts? Are they throwing exceptions? reporting warnings? etc. Obviously, tools can be updated to deal with this stuff, but certain categories of issues we see occurring in the ecosystem may push us toward one or the other solution. Quick Info, for example, seems to drop the subsequent It's not obvious to me if one specific order for concatenation is correct. Maybe the most straightforward thing is to use whatever order we already use for attributes, which appears to be "definition then implementation". SharpLab. In non-private cases where the member is more likely to be documented, the definition part is more likely to be user-authored, so putting it first seems good. |
Version Used:
4.2.0-3.22181.8 (a59a22c)
Steps to Reproduce:
Expected Behavior:
The XML comments from both partials will be used.
Actual Behavior:
Only the XML comments from one or the other partial is used. If you look at the generated .xml file, for example, it includes just the "world" remarks from the second M():
cc: @RikkiGibson
The text was updated successfully, but these errors were encountered: