Adds XML Comments to FileSystemProxy and SpecialDirectoriesProxy

[paul1956]

Addresses adding XML comment #11984

Proposed changes

Add XML Comments to FileSystemProxy and SpecialDirectoriesProxy, they are together because FileSystem references SpecialDirectories.

Customer Impact

• Easier development when IO functions have XML comments

Regression?

• No

Risk

• None no code changes

Microsoft Reviewers: Open in CodeFlow

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.43008%. Comparing base (0d0e1c9) to head (d3b7d53).

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #12141         +/-   ##
===================================================
+ Coverage   75.42712%   75.43008%   +0.00296%     
===================================================
  Files           3102        3102                 
  Lines         634244      634247          +3     
  Branches       46866       46866                 
===================================================
+ Hits          478392      478413         +21     
+ Misses        152434      152422         -12     
+ Partials        3418        3412          -6     

Flag	Coverage Δ
Debug	75.43008% <100.00000%> (+0.00296%)	⬆️
integration	17.98468% <0.00000%> (+0.00510%)	⬆️
production	48.81056% <100.00000%> (+0.00686%)	⬆️
test	97.02624% <ø> (ø)
unit	45.81884% <100.00000%> (-0.01703%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[lonitra]

Let's add < see cref > when we are referring to specific type or value in the description

[lonitra]

Suggested change

	''' Delete the given directory, with options to recursively delete, show progress UI,
	''' Deletes the given directory, with options to recursively delete, show progress UI,

[paul1956]

Let's add < see cref > when we are referring to specific type or value in the description

@lonitra
Do you add cref to all instances, or only the first, or only in return/vslue? The repo seems to have a mix of all 3 options?

[lonitra]

Do you add cref to all instances, or only the first, or only in return/vslue? The repo seems to have a mix of all 3 options?

yes unfortunately, we have some inconsistencies here. Ideally all instances cref should be added when a specific value is being referred to in documentation xml -- especially if it is public

[paul1956]

@lonitra i have no issue doing that but it will really clutter up the comments. There are very few cases where that is done, usually if it’s in a parameter/return/value it’s not repeated in the summary. VS has a quick fix if they are fully qualified otherwise you need to qualify first and then do CodeFix. I thought about an analyzer for this but it would not be an insignificant effort. Maybe a modification of existing CodeFix to add an Analyzer would be possible.

For things like ComboBox that are used multiple times in a sentence/paragraph would you on do the first instance in the section only or all?

[lonitra]

I see, good point. If it will clutter things up making readability difficult then let's just make sure that there is a cref for the first time the value is referenced so that it is easy to navigate to if readers are curious to know more information about the value.

[paul1956]

I see, good point. If it will clutter things up making readability difficult then let's just make sure that there is a cref for the first time the value is referenced so that it is easy to navigate to if readers are curious to know more information about the value.

@paul1956 I agree the first use would be best but the majority of existing comments put the cref in the value or return section if there is only 1 cref. Moving it is easy but should be a separate PR. Maybe we should open a Tracking issue?

I can fix this PR to follow these guidelines today.

[lonitra]

I agree the first use would be best but the majority of existing comments put the cref in the value or return section if there is only 1 cref.

I'm not sure I follow this, could you share an example of what you are referring to? Is the issue that it is referenced in the summary but cref exists only in value or return section?

Moving it is easy but should be a separate PR.

I agree, a separate PR focusing on this would be easier to review.

[paul1956]

I agree the first use would be best but the majority of existing comments put the cref in the value or return section if there is only 1 cref.

I'm not sure I follow this, could you share an example of what you are referring to? Is the issue that it is referenced in the summary but cref exists only in value or return section?

@lonitra

    /// <summary>
    ///  Returns the value of the specified <paramref name="propertyID"/> from the
    ///  element in the form of a <see cref="VARIANT"/>. See
    ///  <see href="https://learn.microsoft.com/windows/win32/winauto/uiauto-automation-element-propids">
    ///   which outlines how the <see cref="VARIANT"/> should be defined for each <see cref="UIA_PROPERTY_ID"/>
    ///  </see>
    /// </summary>
    /// <param name="propertyID">Identifier indicating the property to return.</param>
    /// <returns>The requested value if supported or <see cref="VARIANT.Empty"/> if it is not.</returns>

Just 1 of many examples where see cref is repeated multiple times even in one section (twice in summary) and once in returns.

        ''' <summary>
        '''  Gets the description associated with the assembly.
        ''' </summary>
        ''' <value>
        '''  String containing the <see cref="AssemblyDescriptionAttribute"/>
        '''  associated with the assembly.
        ''' </value>

Examples where it is only in value but it could be in summary (I like this one).

    /// <summary>
    ///  Gets the current <see cref="HighDpiMode"/> mode for the process.
    /// </summary>
    /// <value>One of the enumeration values that indicates the high DPI mode.</value>

Example with it is only in summary.

[Tanya-Solyanik]

We don't own implementations, we are only exposing thin wrappers, so documenting specific path strings will not be up-to-us and might not be accurate.
@KlausLoeffelmann - what do you think?

[paul1956]

@Tanya-Solyanik this should be ready to review with the updated abbreviated comments. The stuff needing cref has been deleted per your feedback.

[paul1956]

@lonitra after you approved Tanya-Solyanik had comments that need fixing. Could I get another review, I fixed all issues.

[lonitra]

@paul1956 Thank you for addressing comments!
It looks like we are waiting for some feedback. @KlausLoeffelmann PTAL in regards to #12141 (review)

[paul1956]

@Tanya-Solyanik added cref's ready to review. Some of the comments were updated because VB does not expose all the same options the underlaying code does.