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

Razor implicit/explicit expressions in HTML attributes are not colored properly #20140

Closed
NTaylorMullen opened this issue Mar 25, 2020 · 2 comments · Fixed by dotnet/razor#1824
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Milestone

Comments

@NTaylorMullen
Copy link
Contributor

Our TextMate grammar doesn't look like it properly supports Razor expressions in HTML attributes.

Easiest way to repro is to create a new Blazor project and then open NavMenu.razor. There's this line that doesn't colorize correctly:

<div class="@NavMenuCssClass" @onclick="ToggleNavMenu">
@NTaylorMullen NTaylorMullen added bug This issue describes a behavior which is not expected - a bug. cost: S area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Mar 25, 2020
@NTaylorMullen NTaylorMullen added this to the Razor LSP Preview 3 milestone Mar 25, 2020
@NTaylorMullen NTaylorMullen self-assigned this Mar 25, 2020
@NTaylorMullen NTaylorMullen changed the title Razor implicit expressions in HTML attributes are not colored properly Razor implicit/explicit expressions in HTML attributes are not colored properly Mar 25, 2020
NTaylorMullen added a commit to dotnet/razor that referenced this issue Mar 25, 2020
- This makes our new Razor LSP editor bits look almost identical to the existing VS experience.
- There are several issues in regards to C# in attributes and that's tracked here: dotnet/aspnetcore#20140
- There's no semantic colorization for tags or C# yet either so that breaks down the colorization expected for our Blazor components and TagHelpers.
- Had to name the textmate grammar with the specific name of `aspnetcorerazor.tmLanguage.tmTheme` in order to enable the VS themeing support to locate our theme.
- One big colorization difference that can't currently be supported is the background coloring of our C# bits. This is something VS' CoreEditor is willing to change on our behalf though.

Fixes dotnet/aspnetcore#18769
@NTaylorMullen
Copy link
Contributor Author

This impacts the immediate colorization experience of the Razor editor in default templates. Pulling into our alpha release.

@NTaylorMullen NTaylorMullen removed their assignment Mar 25, 2020
NTaylorMullen added a commit to dotnet/razor that referenced this issue Mar 26, 2020
- This makes our new Razor LSP editor bits look almost identical to the existing VS experience.
- There are several issues in regards to C# in attributes and that's tracked here: dotnet/aspnetcore#20140
- There's no semantic colorization for tags or C# yet either so that breaks down the colorization expected for our Blazor components and TagHelpers.
- Had to name the textmate grammar with the specific name of `aspnetcorerazor.tmLanguage.tmTheme` in order to enable the VS themeing support to locate our theme.
- One big colorization difference that can't currently be supported is the background coloring of our C# bits. This is something VS' CoreEditor is willing to change on our behalf though.

Fixes dotnet/aspnetcore#18769
@TanayParikh TanayParikh self-assigned this Apr 15, 2020
@TanayParikh TanayParikh added the Done This issue has been fixed label Apr 27, 2020
@TanayParikh
Copy link
Contributor

FYI: @ryanbrandenburg @noahbkim

Sample Text to try this out during validations (screenshots of before / after are in the PR).

Explicit

<div class="@(NavMenuCssClass)"></div>
<button @onclick="@(ToggleNavMenu())"></button>
<button @onclick="@()"></button>
<button @onclick="@(DateTime.Now)"></button>
<button @onclick="@(456 + new Array<int>(){1,2,3}[0] + await GetValueAsync<string>() ?? someArray[await DoMoreAsync(() => {})])"></button>

<button @onclick="@(
    Html.BeginForm(
        "Login",
        "Home",
        new
        {
            @class = "someClass",
            notValid = Html.DisplayFor<object>(
                (_) => Model,
                "name",
                "someName",
                new { })
        })
)"></button>

Implicit

<div class="@NavMenuCssClass"></div>
<button @onclick="@ToggleNavMenu()"></button>
<button @onclick="ToggleNavMenu"></button>
<button @onclick="abc@DateTime.Now"></button>
<button @onclick=")@DateTime.Now"></button>
<button @onclick=".@DateTime.Now"></button>
<button @onclick="}@DateTime.Now"></button>
<button @onclick="@"></button>
<button @onclick="@DateTime.Now{"></button>
<button @onclick="@DateTime.Now}"></button>
<button @onclick="@DateTime.Now)"></button>
<button @onclick="@DateTime.Now]"></button>
<button @onclick="@DateTime.Now"></button>
<button @onclick="@await AsyncProperty"></button>
<button @onclick="@await AsyncMethod()"></button>
<button @onclick="@DateTime!.Now()[1234 + 5678](abc()!.Current, 1 + 2 + getValue())?.ToString[123](() => 456)"></button>
<button @onclick="<p>@DateTime.Now</p>"></button>

<button @onclick="@DateTime!.Now()[1234 +
        5678](
        abc()!.Current,
        1 + 2 + getValue())?.ToString[123](
        ()=>
    {
    var x = 123;
    var y = true;

    return y ? x : 457;
    })">
</button>

@ghost ghost locked as resolved and limited conversation to collaborators Jun 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants