Fix F1 keyword for interpolated and verbatim strings by Youssef1313 · Pull Request #46987 · dotnet/roslyn

Youssef1313 · 2020-08-20T07:37:43Z

$_CSharpKeyword: This article is what have this f1 keyword.

@_CSharpKeyword: This article is what have this f1 keyword.

I removed the following part:

            if (token.IsKind(SyntaxKind.PlusEqualsToken))
            {
                text = "+=_CSharpKeyword";
                return true;
            }

            if (token.IsKind(SyntaxKind.MinusEqualsToken))
            {
                text = "-=_CSharpKeyword";
                return true;
            }

These if conditions are impossible to be met because of the check SyntaxFacts.IsAssignmentExpressionOperatorToken before them. SyntaxFacts.IsAssignmentExpressionOperatorToken will already return true for MinusEqualsToken and PlusEqualsTokene.

~~It even seem that SyntaxFacts.IsAssignmentExpressionOperatorToken is also redundant, this check is already done in IsOperator.~~ (This was incorrect, I'll revert it)

Youssef1313 · 2020-08-20T07:54:10Z

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

+            if (token.IsKind(SyntaxKind.InterpolatedStringStartToken) ||
+                token.IsKind(SyntaxKind.InterpolatedStringEndToken) ||
+                token.IsKind(SyntaxKind.InterpolatedStringTextToken))
+            {
+                text = "$_CSharpKeyword";
+                return true;
+            }
+
+            if (token.IsKind(SyntaxKind.StringLiteralToken) && token.ValueText[0] == '@')
+            {
+                text = "@_CSharpKeyword";
+                return true;
+            }


Currently, the dotnet/docs doesn't have any @$_CSharpKeyword F1 keyword. I opened a PR (dotnet/docs#20197) to add this in docs. Are you okay with this new F1 keyword and adding the following here?

if (token.IsKind(SyntaxKind.InterpolatedVerbatimStringStartToken)) { text = "@$_CSharpKeyword"; return true; }

@davidwengier Should I add the combination?

Looks like that would regress the experience from a help page (albeit for string, which isn't great) to a generic 404 page, so I'd say leave it until after the doc page is created.

@davidwengier I've a PR there
dotnet/docs#20197, so if you agree for the combination@$, we can wait for this PR until the docs one is merged.

Youssef1313 · 2020-08-20T11:49:27Z

        [WorkItem(867572, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/867572")]
        [Fact, Trait(Traits.Feature, Traits.Features.F1Help)]
        public async Task TestSubscription()
        {
            await TestAsync(
@"class CCC
{
    event System.Action e;
    void M()
    {
        e +[||]= () => {
        };
    }
}", "CCC.e.add");
        }

This test failed unintentionally with my changes. But actually the new behavior is what makes sense. The result should be +=_CSharpKeyword instead of CCC.e.add.

CyrusNajmabadi · 2020-08-20T14:25:31Z

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

+                token.IsKind(SyntaxKind.InterpolatedStringTextToken))
            {
-                text = "::_CSharpKeyword";
+                text = "$_CSharpKeyword";


can you provide the links these now go to.

The $_CSharpKeyword should go to this:

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/interpolated

and is that where $_CSharpKeyword does redirect to? Can you give me the links these produce directly, so i can validate they end up landing on an appropriate page? Thanks!

https://msdn.microsoft.com/query/dev16.query?appId=Dev16IDEF1&l=EN-US&k=k($_CSharpKeyword);k(DevLang-csharp)&rd=true

That link takes me to https://docs.microsoft.com/en-us/.

Also, adding @davidwengier as he touched this area before (dotnet/docs#19700 (comment))

It actually redirects to "/Areas/Epx/Content/500.aspx?aspxerrorpath=/query/dev16.query" first, which in turn redirected to the home docs.
So, it's an internal server error.

The query service seems to be pretty flaky at the moment, not sure what the deal is with that. The link above (msdn.microsoft.com/query/dev16.query?appId=Dev16IDEF1&l=EN-US&k=k($_CSharpKeyword);k(DevLang-csharp)&rd=true) worked for me, and looks how I would expect.

this link still doesn't seem to work.

@CyrusNajmabadi Currently it's working with me. The service was down yesterday but is working now, maybe there is some caching issues?

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

davidwengier

Signing off as far as the help links go, I don't see any issues introduced here. The => issue I mentioned looks like it is pre-existing so no reason to block this PR, but it would be nice to fix if you're in the mood :)

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

davidwengier · 2020-08-20T22:47:55Z

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

+                token.IsKind(SyntaxKind.InterpolatedStringTextToken))
            {
-                text = "::_CSharpKeyword";
+                text = "$_CSharpKeyword";


The query service seems to be pretty flaky at the moment, not sure what the deal is with that. The link above (msdn.microsoft.com/query/dev16.query?appId=Dev16IDEF1&l=EN-US&k=k($_CSharpKeyword);k(DevLang-csharp)&rd=true) worked for me, and looks how I would expect.

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

Youssef1313 · 2020-08-21T15:27:28Z

This is ready for review again after applying suggestions from @davidwengier.

Youssef1313 · 2020-08-23T20:18:58Z

Pinging @davidwengier
Could you approve the docs PR?
Then, this should be ready to go.

davidwengier · 2020-08-24T00:14:29Z

I think its really up to the docs team to approve that PR, if they're happy with where that keyword goes, or if they think it should go to one of the specific operator pages, or indeed a new page.

CyrusNajmabadi · 2020-08-24T03:41:07Z

The link still isn't working for me :-/

Youssef1313 · 2020-08-24T05:41:23Z

@CyrusNajmabadi What if you press F1 anything inside Visual Studio? (await keyword for example) Does it work?

Youssef1313 · 2020-08-26T16:08:55Z

@davidwengier The PR that adds @$_CSharpKeyword is now merged.

davidwengier

LGTM

Youssef1313 · 2020-08-27T21:13:54Z

@davidwengier Anything left to merge this?

davidwengier · 2020-08-27T23:05:41Z

@CyrusNajmabadi are you okay with this?

CyrusNajmabadi · 2020-08-27T23:17:14Z

Yup. Thanks!

Youssef1313 added 2 commits August 20, 2020 09:37

Fix F1 keyword for interpolated and verbatim strings

48ae86f

Update F1HelpTests.cs

0b5f542

Youssef1313 mentioned this pull request Aug 20, 2020

Add @$_CSharpKeyword F1 keyword to tokens index dotnet/docs#20197

Merged

Youssef1313 commented Aug 20, 2020

View reviewed changes

Youssef1313 marked this pull request as ready for review August 20, 2020 08:19

Youssef1313 requested a review from a team as a code owner August 20, 2020 08:19

Youssef1313 added 3 commits August 20, 2020 12:07

Swap checking order

55ab67d

Update CSharpHelpContextService.cs

4825e84

Update CSharpHelpContextService.cs

0d467f9

Youssef1313 added 2 commits August 20, 2020 15:26

Revert bad change

592a1f5

Update TestSubscription to the correct expected value

0b67f74

CyrusNajmabadi reviewed Aug 20, 2020

View reviewed changes

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs Show resolved Hide resolved

jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Aug 20, 2020

jinujoseph assigned CyrusNajmabadi Aug 20, 2020

davidwengier approved these changes Aug 20, 2020

View reviewed changes

Youssef1313 added 3 commits August 21, 2020 08:23

Move interpolation and verbatim from operators to new method

a931c7b

Switch to namedParameter_CSharpKeyword and use IsKind()

a077b94

Update CSharpHelpContextService.cs

03a9a8a

Youssef1313 requested review from CyrusNajmabadi and davidwengier August 21, 2020 15:26

Youssef1313 closed this Aug 21, 2020

Youssef1313 reopened this Aug 21, 2020

Youssef1313 marked this pull request as draft August 21, 2020 15:34

Youssef1313 marked this pull request as ready for review August 21, 2020 15:34

Youssef1313 added 2 commits August 21, 2020 20:51

Update CSharpHelpContextService.cs

f1ad4e6

Update F1HelpTests.cs

fb6f8d7

Youssef1313 marked this pull request as draft August 22, 2020 14:24

Youssef1313 marked this pull request as ready for review August 22, 2020 14:24

davidwengier approved these changes Aug 26, 2020

View reviewed changes

CyrusNajmabadi merged commit f3ea6a2 into dotnet:master Aug 27, 2020

ghost added this to the Next milestone Aug 27, 2020

Youssef1313 deleted the patch-9 branch August 28, 2020 06:23

allisonchou modified the milestones: Next, 16.8.P3 Aug 31, 2020

davidwengier mentioned this pull request Feb 15, 2022

Provide f1 help for discards #59543

Merged

Conversation

Youssef1313 commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Youssef1313 commented Aug 20, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwengier Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidwengier Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Youssef1313 commented Aug 21, 2020

Uh oh!

Youssef1313 commented Aug 23, 2020

Uh oh!

davidwengier commented Aug 24, 2020

Uh oh!

CyrusNajmabadi commented Aug 24, 2020

Uh oh!

Youssef1313 commented Aug 24, 2020

Uh oh!

Youssef1313 commented Aug 26, 2020

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

Youssef1313 commented Aug 27, 2020

Uh oh!

davidwengier commented Aug 27, 2020

Uh oh!

CyrusNajmabadi commented Aug 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Youssef1313 commented Aug 20, 2020 •

edited

Loading

Youssef1313 Aug 20, 2020 •

edited

Loading

Youssef1313 Aug 21, 2020 •

edited

Loading

davidwengier Aug 20, 2020 •

edited

Loading

davidwengier Aug 20, 2020 •

edited

Loading