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

Add new line before 'where' constraints in Quick Info #60545

Merged
merged 7 commits into from
Nov 12, 2024

Conversation

Youssef1313
Copy link
Member

Closes #5 (the oldest open issue 😄)

  • Tests to be added.
  • VB to be fixed as well.

Open question:

  • I could have done this in SymbolDisplay in the compiler layer. But I'm not sure if this would be okay.

  • If this is to be done in the compiler layer, it can be done in two ways:

    • Always add line break.
    • Introduce a new SymbolDisplayOptions option, for example SymbolDisplayMiscellaneousOptions.LineBreakBeforeTypeParameterConstraint (will be a new public API).

The current approach is to do the work in IDE only.

@Youssef1313 Youssef1313 requested a review from a team as a code owner April 3, 2022 07:23
@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Apr 3, 2022
@CyrusNajmabadi
Copy link
Member

Pics please. For simple and complex cases.

@Youssef1313
Copy link
Member Author

Testing revealed that this doesn't handle named types. Named types take a different code path (AddSymbolDescription(INamedTypeSymbol symbol)). Here is how it looks like for methods:

image

I'll adjust the implementation and add unit tests once an approach is decided:

@DoctorKrolic
Copy link
Contributor

@Youssef1313 Looking at your screenshot, I would say, that you should add some offset (\t for instance) before each where line to make it even more readable

@Youssef1313
Copy link
Member Author

Youssef1313 commented Jun 23, 2022

Thanks @DoctorKrolic
Updated it to look like this:

image

@CyrusNajmabadi Let me know what do you think

@sharwell
Copy link
Member

Can you also add a test where someone has a comment like this?

/// <summary>
/// This is some text <see langword="where"/> I shouldn't have a line break.
/// </summary>

@sharwell sharwell marked this pull request as draft June 23, 2022 14:38
@DoctorKrolic
Copy link
Contributor

@Youssef1313 Can you please revisit this PR. When a class has really many wheres it gets as ridiculous as this:
Npn9h4xeCf
I don't know how about you, but I can barely read this mess...

@Youssef1313
Copy link
Member Author

@DoctorKrolic I'm getting a little bit busy as my university exams are approaching. :(
I don't mind if you want to take this up and open a PR (feel free to cherry-pick the commit or copy the code here and continue on it, if you want)

@CyrusNajmabadi
Copy link
Member

@Youssef1313 do you want to continue this pr?

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review November 12, 2024 17:27
@CyrusNajmabadi
Copy link
Member

I'll take over this PR.

@@ -478,28 +501,6 @@ private void AddDescriptionForNamedType(INamedTypeSymbol symbol)
}
}

private void AddSymbolDescription(INamedTypeSymbol symbol)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inlined this.

@CyrusNajmabadi CyrusNajmabadi merged commit 9980db3 into dotnet:main Nov 12, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Nov 12, 2024
@Youssef1313 Youssef1313 deleted the symbol-display-constraints branch November 12, 2024 19:57
@Youssef1313
Copy link
Member Author

Thank you @CyrusNajmabadi. Sorry I wasn't able to continue it. Lots of stuff taking up my time :(

@CyrusNajmabadi
Copy link
Member

Not a problem at all :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intellisense Tooltip get too long in case of many generic parameters
5 participants