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

Support wrapping comment lines #1355

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

make1980
Copy link

@make1980 make1980 commented Sep 26, 2024

Support wrapping single line comment and multi-line comments based on line width. This commit fixes #1352

@make1980 make1980 marked this pull request as draft September 26, 2024 22:39
@make1980 make1980 marked this pull request as ready for review September 26, 2024 22:40
@belav
Copy link
Owner

belav commented Sep 27, 2024

There are a whole lot of edge cases that need to get addressed. This changed too many files to easily review it via the web link. I added a few of them below. - https://github.com/belav/csharpier-repos/pull/114/files#diff-a6ea5fd61ca05285c0b3163f7a11ca3aadea54eb5a2f77ad10fc2a7f0cab86d1

The last one is concerning, if someone comments out a big block of code, it is reformatted with line breaks, and then the uncomment it, will it still be valid code? I believe that is true.

There are also failing tests to look at.

I didn't look too closely at the code, but did see a lot of explicit variable types. var needs to be used everywhere it is possible.

Documentation comments end up with a // instead of ///

        /// <param name="messageFormat">A composite format string explaining the reason for the exception.</param>

        /// <param name="messageFormat">A composite format string explaining the reason for the
        // exception.</param>

Should this be combining short comments into a single comment? Otherwise there is no way to avoid this.

                // Documentation does not state the behavior when the stream is closed. The NetworkStream
                // implementation suggests the contract should be to throw ObjectDisposedException when the stream is
                // closed.

                // Documentation does not state the behavior when the stream is closed. The NetworkStream
                // implementation suggests the contract should be to throw ObjectDisposedException when the stream
                // is
                // closed.

But if we do start combining lines, then this is going to get clobbered together. And also combining lines is going to really screw with commented out code.

// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

-- with this PR formats as
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license
// information.

-- if lines are combined, it would end up something like
// Copyright (c) .NET Foundation. All rights reserved. Licensed under the Apache License, Version
// 2.0. See License.txt in the project root for license information.

This is one examble of what it does to commented out code

        //[Variation("ReadSubtree Test on Root", Pri = 0, Params = new object[]{"root", "", "ELEMENT", "", "", "NONE" })]
        //[Variation("ReadSubtree Test depth=1", Pri = 0, Params = new object[] { "elem1", "", "ELEMENT", "elem5", "", "ELEMENT" })]
        //[Variation("ReadSubtree Test depth=2", Pri = 0, Params = new object[] { "elem2", "", "ELEMENT", "elem1", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test depth=3", Pri = 0, Params = new object[] { "x:elem3", "", "ELEMENT", "elem2", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test depth=4", Pri = 0, Params = new object[] { "elem4", "", "ELEMENT", "x:elem3", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test empty element", Pri = 0, Params = new object[] { "elem5", "", "ELEMENT", "elem6", "", "ELEMENT" })]
        //[Variation("ReadSubtree Test empty element before root", Pri = 0, Params = new object[] { "elem6", "", "ELEMENT", "root", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test PI after element", Pri = 0, Params = new object[] { "elempi", "", "ELEMENT", "pi", "target", "PROCESSINGINSTRUCTION" })]
        //[Variation("ReadSubtree Test Comment after element", Pri = 0, Params = new object[] { "elem", "", "ELEMENT", "", "Comment", "COMMENT" })]


        //[Variation("ReadSubtree Test on Root", Pri = 0, Params = new object[]{"root", "", "ELEMENT", "",
        // "", "NONE" })]
        //[Variation("ReadSubtree Test depth=1", Pri = 0, Params = new object[] { "elem1", "", "ELEMENT",
        // "elem5", "", "ELEMENT" })]
        //[Variation("ReadSubtree Test depth=2", Pri = 0, Params = new object[] { "elem2", "", "ELEMENT",
        // "elem1", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test depth=3", Pri = 0, Params = new object[] { "x:elem3", "", "ELEMENT",
        // "elem2", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test depth=4", Pri = 0, Params = new object[] { "elem4", "", "ELEMENT",
        // "x:elem3", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test empty element", Pri = 0, Params = new object[] { "elem5", "",
        // "ELEMENT", "elem6", "", "ELEMENT" })]
        //[Variation("ReadSubtree Test empty element before root", Pri = 0, Params = new object[] { "elem6",
        // "", "ELEMENT", "root", "", "ENDELEMENT" })]
        //[Variation("ReadSubtree Test PI after element", Pri = 0, Params = new object[] { "elempi", "",
        // "ELEMENT", "pi", "target", "PROCESSINGINSTRUCTION" })]
        //[Variation("ReadSubtree Test Comment after element", Pri = 0, Params = new object[] { "elem", "",
        // "ELEMENT", "", "Comment", "COMMENT" })]

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 this pull request may close these issues.

Comments are not automatically wrapped.
2 participants