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

Unify the final state management processing in inline-rename. #61512

Merged
merged 5 commits into from
May 26, 2022

Conversation

CyrusNajmabadi
Copy link
Member

Followup to #61509.

This takes the 3 codepaths that end up ending the rename-session and get them to use one common cleanup routine. WIll doc inline.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 25, 2022 18:32
@@ -367,34 +365,6 @@ public void SetPreviewChanges(bool value)
_previewChanges = value;
}

private void Dismiss(bool rollbackTemporaryEdits)
Copy link
Member Author

Choose a reason for hiding this comment

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

Dismiss was called from one location only. So it was inlined as a local function in that one place to make sure nothing else calls it directly. It was entirely unchanged.


// We're about to perform the final commit action. No need to do any of our BG work to find-refs or compute conflicts.
_cancellationTokenSource.Cancel();
_conflictResolutionTaskCancellationSource.Cancel();
Copy link
Member Author

Choose a reason for hiding this comment

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

these two cancels were in CancelAllOpenDocumentTrackingTasks. This was the only place that called that, so this was inlined here.

private void DismissUIAndRollbackEditsAndEndRenameSession(
RenameLogMessage.UserActionOutcome outcome,
bool previewChanges,
Action finalCommitAction = null)
Copy link
Member Author

Choose a reason for hiding this comment

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

two codepaths are just ones that dismiss/cleanup without committing. And one codepath actually has some commit logic it performs before final cleanup. So i represented this as this single method with the optional commit action to perform.

RenameTrackingDismisser.DismissRenameTracking(_workspace, _workspace.GetOpenDocumentIds());

// Log how long the full rename took.
_inlineRenameSessionDurationLogBlock.Dispose();
Copy link
Member Author

Choose a reason for hiding this comment

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

The above code is waht was in EndRenameSession. It was only called here, so i inlined as well.

catch (InvalidOperationException ex) when (FatalError.ReportAndCatch(ex))
{
// Unreachable code due to ReportAndCatch
Contract.ThrowIfTrue(true);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

just removed this as it's not needed at all.

@CyrusNajmabadi CyrusNajmabadi enabled auto-merge May 25, 2022 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants