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

Verbatim string literals take into account EndOfLine configuration #183

Closed
belav opened this issue May 6, 2021 · 3 comments
Closed

Verbatim string literals take into account EndOfLine configuration #183

belav opened this issue May 6, 2021 · 3 comments
Assignees
Labels
area:formatting type:bug Something isn't working
Milestone

Comments

@belav
Copy link
Owner

belav commented May 6, 2021

The file SyntaxNodeComparerTests contained \r\n inside of multiline strings.
CSharpier is set to format the file with \n as line endings
It formatted the file with \n, but not inside of the multiline strings.
This lead to the github action for validating that the files were formatted to fail, because of a mismatch between \r\n and \n on some lines.

@belav belav added this to the 0.9.2 milestone May 6, 2021
@shocklateboy92
Copy link
Collaborator

shocklateboy92 commented May 9, 2021

Oaf, that's a tough one. Yes people should normalize their line endings, but if we mess with strings we're potentially changing target code's behavior.

That should probably be a big no-no.

@belav
Copy link
Owner Author

belav commented May 14, 2021

My original description wasn't super clear since I was mostly in a panic about why things appeared borked with the 0.9.1 release.
The problem is with verbatim strings like this.

            ResultShouldBe(
                result,
                @"    Original: Around Line 0
class ClassName { }
    Formatted: Around Line 0
namespace Namespace { }
"
            );

When I check out a file with this code on windows, the file ends up with \r\n at the end of each line, including the lines inside of the string.
When I formatted the file using csharpier and EndOfLine= "LF" then csharpier left \r\n at the end of the lines in the string, but used \n on every other line in the file.

For verbatim strings, I believe that csharpier should respect the EndOfLine setting for the actual end of each line of the verbatim string.
For strings (including verbatim) of the form "someString\r\n", I don't believe csharpier should touch the \r\n

@belav
Copy link
Owner Author

belav commented May 14, 2021

I have two possible fixes in #191 but I'm not happy with either.

Related, right now lineEndings are affecting print width. Because the width of the string is calculated by the characters in it so the following is considered to have a different length depending on if the line endings in the verbatim string are \n or \r\n

var value = @"one
two";

Which leads to a file formatting differently if the verbatim string is just the right length. Like the code in LineEndings_Should_Not_Affect_Printed_Output

And to make things even more complicated. How should the following format? Do we want to calculate the width of a verbatim string based on the first "line" of it?

class ClassName
{
    // should this @ stay on the same line because the first "line" of the string is short?
    private string blah = @"one
two
three
four
jhaksdlfklasdfjlkasdjflaksdfklasldkjfljkasdljfkasljkdfakljsdfjlkaskfjlaskjldfksdjlf
";

    // this is short enough that it stays on the same line
    private string blah = @"one
two
three
four";

    // either way, this would break because the first line is too long, and the whole string is too long
    private string blah =
        @"jhaksdlfklasdfjlkasdjflaksdfklasldkjfljkasdljfkasljkdfakljsdfjlkaskfjlaskjldfksdjlf
two
three
four
";

    // then what about methods? and other areas verbatim strings could appear
    private void MethodName()
    {
        CallSomeLongMethod(
            @"one
two
three
four
jhaksdlfklasdfjlkasdjflaksdfklasldkjfljkasdljfkasljkdfakljsdfjlkaskfjlaskjldfksdjlf
"
        );

        CallSomeLongMethod(@"one
two
three
four");

        CallSomeLongMethod(
            @"jhaksdlfklasdfjlkasdjflaksdfklasldkjfljkasdljfkasljkdfakljsdfjlkaskfjlaskjldfksdjlf
two
three
four
"
        );
    }
}

@belav belav changed the title EndOfLine - should this affect multiline strings? EndOfLine + Verbatim strings May 14, 2021
@belav belav self-assigned this May 14, 2021
@belav belav added type:bug Something isn't working area:formatting labels May 14, 2021
@belav belav closed this as completed in f7380db May 17, 2021
@belav belav changed the title EndOfLine + Verbatim strings Verbatim string literals take into account EndOfLine configuration May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:formatting type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants