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

Comment on raw string literal is lost when file is formatted. #937

Closed
Tyrrrz opened this issue Aug 22, 2023 · 5 comments · Fixed by #939
Closed

Comment on raw string literal is lost when file is formatted. #937

Tyrrrz opened this issue Aug 22, 2023 · 5 comments · Fixed by #939
Milestone

Comments

@Tyrrrz
Copy link
Contributor

Tyrrrz commented Aug 22, 2023

Repro: run dotnet build on Tyrrrz/YoutubeExplode@78680d3

Here's the build log: https://github.com/Tyrrrz/YoutubeExplode/actions/runs/5942894160/job/16116827695

image

As you can see, it errors out on the // lang=json comment for some reason.

Here's the code it fails on:

https://github.com/Tyrrrz/YoutubeExplode/blob/78680d3f087285767f96d170a46f70bae01fa524/YoutubeExplode/Search/SearchController.cs#L21-L51

@belav
Copy link
Owner

belav commented Aug 22, 2023

This failure is coming from the syntax tree validation, because csharpier is eating the // lang=json line. After formatting code it compares the resulting syntax trees to ensure nothing was lost or changed in an unexpected way.

If that doesn't serve a special purpose you could move it above Content = new StringContent( to get things working.

@belav belav added this to the 0.26.0 milestone Aug 22, 2023
@belav belav changed the title CSharpier check errors out on a comment Comment on raw string literal is lost when file is formatted. Aug 22, 2023
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Aug 22, 2023

This failure is coming from the syntax tree validation, because csharpier is eating the // lang=json line. After formatting code it compares the resulting syntax trees to ensure nothing was lost or changed in an unexpected way.

Yeah, that's what I figured. I have similar pieces of code where CSharpier does not eat the same comment. Here, for instance:

https://github.com/Tyrrrz/YoutubeExplode/blob/78680d3f087285767f96d170a46f70bae01fa524/YoutubeExplode/Videos/VideoController.cs#L58-L81

I have a few instances of these, and the only one that causes the issue is the one mentioned in the issue description.

If that doesn't serve a special purpose you could move it above Content = new StringContent( to get things working.

It does actually serve a purpose: it marks the following string literal as a language injected string for JetBrains tools. It allows syntax highlighting inside it for the specified language (in this case, JSON).

https://www.jetbrains.com/help/rider/Language_Injections.html


As an aside, I really like CSharpier! You've done a great job. I've added it to all my projects. I learned about it from this discussion: https://twitter.com/Tyrrrz/status/1693941083896422545

@belav
Copy link
Owner

belav commented Aug 23, 2023

As an aside, I really like CSharpier! You've done a great job. I've added it to all my projects. I learned about it from this discussion: https://twitter.com/Tyrrrz/status/1693941083896422545

Thanks! It has been awesome to see it slowly spreading around twitter/reddit/etc over time.

That second working example made me start to wonder why that first one wasn't working, and it has something to do with the switch expression in the raw string literal. This version should give you a work around for now.

using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using YoutubeExplode.Bridge;

namespace YoutubeExplode.Search;

internal class SearchController
{
    private readonly HttpClient _http;

    public SearchController(HttpClient http) => _http = http;

    public async ValueTask<SearchResponse> GetSearchResponseAsync(
        string searchQuery,
        SearchFilter searchFilter,
        string? continuationToken,
        CancellationToken cancellationToken = default
    )
    {
        var value = searchFilter switch
        {
            SearchFilter.Video => "EgIQAQ%3D%3D",
            SearchFilter.Playlist => "EgIQAw%3D%3D",
            SearchFilter.Channel => "EgIQAg%3D%3D",
            _ => null
        };

        using var request = new HttpRequestMessage(
            HttpMethod.Post,
            "https://www.youtube.com/youtubei/v1/search"
        )
        {
            Content = new StringContent(
                // lang=json
                $$"""
                {
                    "query": "{{searchQuery}}",
                    "params": "{{value}}",
                    "continuation": "{{continuationToken}}",
                    "context": {
                        "client": {
                            "clientName": "WEB",
                            "clientVersion": "2.20210408.08.00",
                            "hl": "en",
                            "gl": "US",
                            "utcOffsetMinutes": 0
                        }
                    }
                }
                """
            )
        };

        using var response = await _http.SendAsync(request, cancellationToken);
        response.EnsureSuccessStatusCode();

        return SearchResponse.Parse(await response.Content.ReadAsStringAsync(cancellationToken));
    }
}

@belav
Copy link
Owner

belav commented Aug 23, 2023

Ah yeah, when a interpolated string contains c# expressions with a new line, CSharpier bails out of trying to format that string. But it didn't keep into account any leading comments when doing so.

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Aug 23, 2023

Nice! Thank you for investigating this :)

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