Remove at most 1 end of line preceding redundant nullable directives #72150
Conversation
ghost
self-requested a review
as a 
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
ghost
added
the
Community
|
Hey, this is still ready for review, I am not sure why the vs integration tests are failing, seems to be a timeout? |
| var nullableDirectivesByNodes = diagnostics.GroupBy(x => | ||
| x.Location.FindNode(findInsideTrivia: false, getInnermostNodeForTie: false, cancellationToken), x => | ||
| x.Location.FindNode(findInsideTrivia: true, getInnermostNodeForTie: true, cancellationToken)) | ||
| .ToDictionary(x => x.Key, x => x.ToList()); |
There was a problem hiding this comment.
what is thsi code for. this needs docs.
| var leadingTrivia = node.GetLeadingTrivia(); | ||
| var indexes = nullableDirectives.Select(x => leadingTrivia.IndexOf(x.ParentTrivia)) | ||
| .OrderByDescending(x => x).ToArray(); | ||
| foreach (var index in indexes) |
There was a problem hiding this comment.
i really don't understand the logic here. including what the above indices are representing, or waht the loop below is doing. this all needs examples.
...emoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs
Show resolved
Hide resolved
...harp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveRedundantNullableDirectiveTests.cs
Show resolved
Hide resolved
...emoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
|
@sdelarosbil do you mind if i push to your repo? if not, i can push to another branch where i show what i mean :) |
you can push to this branch, sure I understand the idea, it's more I think I might miss how to easilly get the trivias relative to the directives that I have a bit of an issue. |
|
Ok. Pushed :) |
|
Oh, I see, yeah now I understand more how you do this now. Thanks! |
This change updates the behavior of AddElasticMarker during syntax node removal to collapse consecutive newlines surrounding the removed node such that the total number of blank lines appearing at the location of the removed node does not increase. This is a dramatic simplification based on dotnet#72150.
Closes #63163
This pr changes the way the redudant directive is removed by checking the trivias preceding them. All of the whitespaces ones are removed until an end of line trivia is found. This means at most, this will delete a blank like preceding the directive which removes a potential blank line.
I added a test, but I needed to change 2 tests to fit the changes. The new algorithm first groups the nodes by their directive nodes which allows to go through each of their parent's childs and perform their removal. This is done on a per node basis so cases where there's multiple directives in a row are batched together.
I am not too sure about the complexity of this and it might be because there's some node apis I wasn't aware of that would have made this simpler, but I at least wanted to present a working solution in case it could be simplified.