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

Multiline comments generation issue #1462

Closed
vadimi opened this issue Dec 13, 2021 · 5 comments · Fixed by #1463
Closed

Multiline comments generation issue #1462

vadimi opened this issue Dec 13, 2021 · 5 comments · Fixed by #1463

Comments

@vadimi
Copy link

vadimi commented Dec 13, 2021

Hello, after PR #1346 code generation for multiline comments produces invalid <summary> xml doc.

For example:

    TestType:
      type: object
      properties:
        code:
          type: string
          description: |
            * test1
            * test2

produces the following c# code:

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.15.3.0 (NJsonSchema v10.6.4.0 (Newtonsoft.Json v13.0.0.0))")]
    public partial class TestType
    {
        /// <summary>* test1
        /// <br/>* test2
    </summary>
        [Newtonsoft.Json.JsonProperty("code", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public string Code { get; set; }

        private System.Collections.Generic.IDictionary<string, object> _additionalProperties = new System.Collections.Generic.Dictionary<string, object>();

        [Newtonsoft.Json.JsonExtensionData]
        public System.Collections.Generic.IDictionary<string, object> AdditionalProperties
        {
            get { return _additionalProperties; }
            set { _additionalProperties = value; }
        }

    }

As you see the closing </summary> tag doesn't start with /// which breaks the compilation

@RicoSuter
Copy link
Owner

RicoSuter commented Dec 14, 2021

Yes, "\n" are not removed anymore which causes this problem...

/cc @lahma any reason why this regression has been introduced?

@kvantetore
Copy link

It seems like the issue was introduced in 463d246 by substituting
.Replace("\n", "\n" + string.Join("", Enumerable.Repeat(" ", tabCount)) + "/// ")
with a call to AddPrefixToBeginningOfNonEmptyLines() in ConversionUtilities.cs line 264.

The AddPrefixToBeginningOfNonEmptyLines() kind of makes sense for the | tab filter, but does not work as intended for the | csharpdocs filter. Imo /// should be added to empty lines as well as non-empty lines.

@lahma
Copy link
Collaborator

lahma commented Dec 14, 2021

I'll have a look.

@lahma
Copy link
Collaborator

lahma commented Dec 14, 2021

Fixed in #1463

@vadimi
Copy link
Author

vadimi commented Dec 16, 2021

thanks for fixing the issue!

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

Successfully merging a pull request may close this issue.

4 participants