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

Errors for invalid formatting use weird characters for spacing #947

Closed
Tyrrrz opened this issue Aug 30, 2023 · 5 comments
Closed

Errors for invalid formatting use weird characters for spacing #947

Tyrrrz opened this issue Aug 30, 2023 · 5 comments
Labels
Milestone

Comments

@Tyrrrz
Copy link
Contributor

Tyrrrz commented Aug 30, 2023

CSharpier appears to use ┬╖┬╖┬╖┬╖┬╖┬╖┬╖┬╖ characters to space out the code snippets in the terminal:

image

It makes the error quite hard to parse visually, and I don't think this is the intended behavior. Any idea why this may be happening?

@belav
Copy link
Owner

belav commented Aug 31, 2023

CSharpier uses the following so that you can visually see the whitespace when code isn't formatted

    private static string? MakeWhiteSpaceVisible(string? value)
    {
        return value?.Replace(' ', '·').Replace('\t', '→');
    }

The goal is to make it obvious if the reason it failed the check is due to whitespace differences. A failure due to extra whitespace like below isn't obvious if you can't see it.

public class ClassName
{
····private string field1;
····
····private string field2;
}

Where are you seeing this output? CSharpier is writing on stdout using UTF8 encoding.

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Aug 31, 2023

I'm seeing this both in GH Actions, as well as locally in Rider's integrated terminal.

Here's the link to the actions job where you can see it: https://github.com/Tyrrrz/YoutubeExplode/actions/runs/6026195051/job/16349413554

May be relevant, dotnet test is executed with these environment variables: https://github.com/Tyrrrz/.github/blob/ee8b7cbb856b44eb0db9c0ec45f305272f7a7a12/.github/workflows/nuget.yml#L44C9-L52

belav added a commit that referenced this issue Sep 1, 2023
@belav
Copy link
Owner

belav commented Sep 1, 2023

Took me a bit - I figured out this is an issue with CSharpier.Msbuild but I believe only on windows.

image

@belav belav added this to the 0.26.0 milestone Sep 1, 2023
belav added a commit that referenced this issue Sep 1, 2023
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 1, 2023

A bit related but slightly off-topic, may I suggest displaying the space indicator characters only for indentation (i.e. before the first non-whitespace character)? It gets a bit noisy when it's in the middle of the line (e.g. return·value?.Replace(... in your example).

@belav
Copy link
Owner

belav commented Sep 2, 2023

@Tyrrrz I can play around with limiting where whitespace is visible. Maybe something like just making the first couple of spaces/tabs that are different will work better and not be so noisy. I wrote up #953 and have a couple of ideas to try out for improving it. Thanks for the suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants