search

belav / csharpier

CSharpier is an opinionated code formatter for c#.
https://csharpier.com
MIT License
1.41k stars 98 forks source link

Support wrapping comment lines #1355

Open make1980 opened 1 month ago

make1980 commented 1 month ago

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

belav commented 1 month ago

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" })]