GH1205: Add support for code actions in Cake #1212
Conversation
bjorkstromm
commented
Jun 4, 2018
bjorkstromm
commented
- Fixes [Cake] Code actions not working #1205
CHANGELOG.md
Outdated
| @@ -1,6 +1,9 @@ | |||
| # Changelog | |||
| All changes to the project will be documented in this file. | |||
|
|
|||
| ## [1.32.0] - _Not Yet Released_ | |||
| * Added support for code actions in `.cake` files. ([#1205](https://github.com/OmniSharp/omnisharp-roslyn/issues/1205)) | |||
There was a problem hiding this comment.
I won the merge race. This will probably have a conflict.
There was a problem hiding this comment.
Good that I caught my eyes on the /codestructure PR. We need to create a Cake endpoint for that too.. Created issue here #1214
| foreach (var change in changes) | ||
| { | ||
|
|
||
| if (!(fileOperations.FirstOrDefault(x => x.FileName == change.Key && |
There was a problem hiding this comment.
You could just invert this to avoid having a continue and an ugly ! before a pattern match. E.g.
if (fileOperations.FirstOrDefault(x => x.FileName == change.Key && x.ModificationType == FileModificationType.Modified) is ModifiedFileResponse modifiedFile)
{
modifiedFile.Changes = change.Value;
}| continue; | ||
| } | ||
|
|
||
| modifiedFile.Changes = change.Value; |
There was a problem hiding this comment.
This means that modifiedFile.Changes will only ever have the last matching value. Is that intentional or should there by some sort of merging?
There was a problem hiding this comment.
No, change is of type KeyValuePair<string, List<LinePositionSpanTextChange>> and the dictionary it's contained in is populated above, using the PopulateModificationsAsync.
This is the same thing we do with rename responses also. it's ugly, but due to the fact that loaded scripts are "merged" into the base script we might end up in a situation where a ModifiledFileResponse contains modifications belonging to a different file. This is what we are trying to fix in this method, e.g. loop through all modifications, map line numbers and assign them to the correct ModifiedFileResponse.
Someday, Cake should not "copy & paste" loaded script contents in the base file and instead use #load as provided by Roslyn scripting.
There was a problem hiding this comment.
Someday, Cake should not "copy & paste" loaded script contents in the base file and instead use #load as provided by Roslyn scripting.
this 👍
There was a problem hiding this comment.
@filipw I see somebody has already written a blog post that will help me getting started 😉 https://www.strathweb.com/2016/06/implementing-custom-load-behavior-in-roslyn-scripting/
bjorkstromm
force-pushed
the
feature/GH-1205
branch
from
b6f5afc to
635e66a
Compare
|
@DustinCampbell addressed feedback and rebased! |
