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

Small bug with formatting LINQ queries with multiple orderby fields #643

Closed
mstijak opened this issue Apr 7, 2022 · 5 comments · Fixed by #646
Closed

Small bug with formatting LINQ queries with multiple orderby fields #643

mstijak opened this issue Apr 7, 2022 · 5 comments · Fixed by #646
Milestone

Comments

@mstijak
Copy link

mstijak commented Apr 7, 2022

The bug is visible in the picture below. There is an extra space after the first field, and the space is missing after the comma. It's really minor, but I wanted to report it anyway.

image

I can give it a try if you point me in the right direction.

@belav
Copy link
Owner

belav commented Apr 7, 2022

If you want to give it a go, the problem is in CSharpier.SyntaxPrinter.SyntaxNodePrinters.OrderByClause

Otherwise I'll get it fixed this weekend. Thanks for reporting it!

@mstijak
Copy link
Author

mstijak commented Apr 8, 2022

I cannot build the project... I installed .NET 5 and changed the version of .NET 6, but it's still not working, so I give up.

Anyway, this was my first attempt:

namespace CSharpier.SyntaxPrinter.SyntaxNodePrinters;

internal static class OrderByClause
{
    public static Doc Print(OrderByClauseSyntax node)
    {
        return Doc.Concat(
            Token.Print(node.OrderByKeyword),
            SeparatedSyntaxList.Print(
                node.Orderings,
                orderingNode =>
                    Doc.Concat(
                        " ",
                        Node.Print(orderingNode.Expression),
                        string.IsNullOrEmpty(orderingNode.AscendingOrDescendingKeyword.Text) ? "" : " ",
                        Token.Print(orderingNode.AscendingOrDescendingKeyword)
                    ),
                Doc.Null
            )
        );
    }
}

I could not test this, so I didn't want to open a PR.

belav added a commit that referenced this issue Apr 11, 2022
@belav
Copy link
Owner

belav commented Apr 11, 2022

Sorry I didn't notice your comment until today. I'm curious what build errors you were seeing. I also ran into some this morning, but I believe mine were due to another branch I have that gets things working with the latest .net6. I had to git clean everything and do a fresh restore and build to get past them.

Your first attempt was spot on, nice work for not being able to test it!

@mstijak
Copy link
Author

mstijak commented Apr 12, 2022

Here it is:
image

The first error I get is:

NuGet package restore failed. Please see Error List window for detailed warnings and errors.
2>C:\Program Files\dotnet\sdk\6.0.201\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets(92,5): error NETSDK1013: The TargetFramework value '' was not recognized. It may be misspelled. If not, then the TargetFrameworkIdentifier and/or TargetFrameworkVersion properties must be specified explicitly.
2>Done building project "Github.csproj" -- FAILED.

After NuGet restore fails, 200+ errors appear.

shocklateboy92 pushed a commit that referenced this issue Apr 12, 2022
@belav
Copy link
Owner

belav commented Apr 25, 2022

There were two issues I found with VS today.

The first was that the build configuration that I set in rider, which is in the .sln, was not being used by VS. So it was trying to build a few projects that aren't meant to be built. I set up the same configuration in VS and now they are both happy.

The second problem is that VS doesn't seem to be respecting the global.json file, which specifies that csharpier should be built with 6.0.100. The source generator fails to build in 6.0.202, which results in a number of other errors. I've updated to the latest VS, and done some googling but can't find anything about the problem. There is an open PR to get things working in 6.0.202 which should be merging today.

@belav belav added this to the 0.17.0 milestone May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants