Skip to content

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

Merged
CyrusNajmabadi merged 7 commits intodotnet:mainfrom
Youssef1313:symbol-display-constraints
Nov 12, 2024
Merged

Add new line before 'where' constraints in Quick Info#60545
CyrusNajmabadi merged 7 commits intodotnet:mainfrom
Youssef1313:symbol-display-constraints

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 Community The pull request was submitted by a contributor who is not a Microsoft employee. Area-IDE labels Apr 3, 2022
@CyrusNajmabadi
Copy link
Contributor

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
Contributor

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
Contributor

@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
Contributor

I'll take over this PR.

}
}

private void AddSymbolDescription(INamedTypeSymbol symbol)
Copy link
Contributor

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
@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
Contributor

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