Give C# the chain terminators it needs - #786
Closed
SimonCropp wants to merge 6 commits into
Closed
Conversation
WalkChain appends a Snapshot call at the end of the chain unless the language names a terminating call to go in front of instead. F# named ToTask and said why: Snapshot returns the SettingsTask and ToTask does not, so appending after it appends to the wrong type. C# named nothing, and has the same shapes. A test that ends its chain by hand - GetAwaiter().GetResult(), ConfigureAwait(false), AsTask() - got the Snapshot appended after the terminator, which does not compile. Worse than not patching at all: the status came back Applied, so the snapshot is recorded as accepted while the file no longer builds. ChainTerminator becomes ChainTerminators, since C# has several, and C# names them. The first one in the chain wins, so GetAwaiter().GetResult() inserts ahead of GetAwaiter rather than between the two.
…terminator # Conflicts: # src/DiffEngine.Tests/InlinePatcherTests.cs
…terminator # Conflicts: # src/DiffEngine.Tests/InlinePatcherTests.cs
…terminator # Conflicts: # src/DiffEngine.Tests/InlinePatcherTests.cs
AsTask is not a member of SettingsTask. I added it from the audit's suggestion without checking, and checking Verify's source says it never existed - so it was a name the patcher would only ever match on somebody's own extension method, and mis-position the insert if it did. The other three are real, public and [Pure] on SettingsTask, and the reason they matter is stronger than "the types do not line up". ToTask sets the task field, and CurrentSettings then throws "This SettingsTask instance has already been converted to a Task and can no longer be modified". So a Snapshot appended after a terminator fails at run time wherever it compiles - and where it does not compile, the patch still reported Applied and the snapshot was recorded as accepted. The test swaps its AsTask case for ToTask, which is the one C# shares with F#.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WalkChain appends a Snapshot call at the end of the chain unless the language
names a terminating call to go in front of instead. F# named ToTask and said
why: Snapshot returns the SettingsTask and ToTask does not, so appending after
it appends to the wrong type.
C# named nothing, and has the same shapes. A test that ends its chain by hand -
GetAwaiter().GetResult(), ConfigureAwait(false), AsTask() - got the Snapshot
appended after the terminator, which does not compile. Worse than not patching
at all: the status came back Applied, so the snapshot is recorded as accepted
while the file no longer builds.
ChainTerminator becomes ChainTerminators, since C# has several, and C# names
them. The first one in the chain wins, so GetAwaiter().GetResult() inserts ahead
of GetAwaiter rather than between the two.