Rmeove unnecessary parameter. Add explanatory comment.

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs

@@ -235,12 +235,7 @@ private void CommitItem(PresentationItem item)
                 return;
             }
 
-            this.Commit(
-                item, this.sessionOpt.Computation.InitialUnfilteredModel,
-                initialTextSnapshot: this.SubjectBuffer.CurrentSnapshot,
-                initialCaretPositionInView: this.TextView.Caret.Position.VirtualBufferPosition,
-                commitChar: null,
-                nextHandler: null);
+            this.CommitOnNonTypeChar(item, this.sessionOpt.Computation.InitialUnfilteredModel);
         }
 
         private const int MaxMRUSize = 10;

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Commit.cs

@@ -29,17 +29,12 @@ private CompletionProvider GetCompletionProvider(CompletionItem item)
         private void CommitOnNonTypeChar(
             PresentationItem item, Model model)
         {
-            Commit(item, model, 
-                commitChar: null,
-                initialTextSnapshot: null, 
-                initialCaretPositionInView: default(VirtualSnapshotPoint),
-                nextHandler: null);
+            Commit(item, model, commitChar: null, initialTextSnapshot: null, nextHandler: null);
         }
 
         private void Commit(
             PresentationItem item, Model model, char? commitChar,
-            ITextSnapshot initialTextSnapshot, VirtualSnapshotPoint initialCaretPositionInView,
-            Action nextHandler)
+            ITextSnapshot initialTextSnapshot, Action nextHandler)
         {
             AssertIsForeground();
 
@@ -83,34 +78,11 @@ private CompletionProvider GetCompletionProvider(CompletionItem item)
                     //      buffer (unless the items asks us not to).  By doing this, we can make sure
                     //      that things like brace-completion or formatting trigger as we expect them
                     //      to.
-                    var currentSnapshot = this.SubjectBuffer.CurrentSnapshot;
                     var characterWasSentIntoBuffer = commitChar != null &&
-                                                     initialTextSnapshot.Version.VersionNumber != currentSnapshot.Version.VersionNumber;
+                                                     initialTextSnapshot.Version.VersionNumber != this.SubjectBuffer.CurrentSnapshot.Version.VersionNumber;
                     if (characterWasSentIntoBuffer)
                     {
-                        // Get all the versions from the initial text snapshot (before we passed the
-                        // commit character down) to the current snapshot we're at.
-                        var versions = GetVersions(initialTextSnapshot, currentSnapshot).ToList();
-
-                        // Un-apply the edits. 
-                        for (var i = versions.Count - 1; i >= 0; i--)
-                        {
-                            var version = versions[i];
-                            using (var textEdit = this.SubjectBuffer.CreateEdit(EditOptions.None, reiteratedVersionNumber: null, editTag: null))
-                            {
-                                foreach (var change in version.Changes)
-                                {
-                                    textEdit.Replace(change.NewSpan, change.OldText);
-                                }
-
-                                textEdit.Apply();
-                            }
-                        }
-
-                        // Move the caret back to where it was prior to committing the item.
-                        //TextView.Caret.MoveTo(new VirtualSnapshotPoint(
-                        //    new SnapshotPoint(this.TextView.TextSnapshot, initialCaretPositionInView.Position.Position),
-                        //    initialCaretPositionInView.VirtualSpaces));
+                        RollbackToBeforeTypeChar(initialTextSnapshot);
                     }
 
                     // Now, get the change the item wants to make.  Note that the change will be relative
@@ -161,6 +133,28 @@ private CompletionProvider GetCompletionProvider(CompletionItem item)
             this.MakeMostRecentItem(item.Item.DisplayText);
         }
 
+        private void RollbackToBeforeTypeChar(ITextSnapshot initialTextSnapshot)
+        {
+            // Get all the versions from the initial text snapshot (before we passed the
+            // commit character down) to the current snapshot we're at.
+            var versions = GetVersions(initialTextSnapshot, this.SubjectBuffer.CurrentSnapshot).ToList();
+
+            // Un-apply the edits. 
+            for (var i = versions.Count - 1; i >= 0; i--)
+            {
+                var version = versions[i];
+                using (var textEdit = this.SubjectBuffer.CreateEdit(EditOptions.None, reiteratedVersionNumber: null, editTag: null))
+                {
+                    foreach (var change in version.Changes)
+                    {
+                        textEdit.Replace(change.NewSpan, change.OldText);
+                    }
+
+                    textEdit.Apply();
+                }
+            }
+        }
+
         private IEnumerable<ITextVersion> GetVersions(
             ITextSnapshot initialTextSnapshot, ITextSnapshot currentSnapshot)
         {

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs

@@ -10,8 +10,6 @@
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.Text;
 using Roslyn.Utilities;
-using System.Threading;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion
 {
@@ -31,21 +29,34 @@ void ICommandHandler<TypeCharCommandArgs>.ExecuteCommand(TypeCharCommandArgs arg
 
             AssertIsForeground();
 
+            // When a character is typed it is *always* sent through to the editor.  This way the
+            // editor always represents what would have been typed had completion not been involved
+            // at this point.  That means that if we decide to commit, then undo'ing the commit will
+            // return you to the code that you would have typed if completion was not up.
+            //
+            // The steps we follow for commit are as follows:
+            //
+            //      1) send the commit character through to the buffer.
+            //      2) open a transaction.
+            //          2a) roll back the text to before the text was sent through
+            //          2b) commit the item.
+            //          2c) send the commit character through again.
+            //          2d) commit the transaction.
+            //
+            // 2c is very important.  it makes sure that post our commit all our normal features
+            // run depending on what got typed.  For example if the commit character was (
+            // then brace completion may run.  If it was ; then formatting may run.  But, importantly
+            // this code doesn't need to know anything about that.  Furthermore, becaue that code
+            // runs within this transaction, then the user can always undo and get to what the code
+            // would have been if completion was not involved.
+            //
+            // In order to support 2a (rolling back), we capture hte state of the buffer before
+            // we send the character through.  We then just apply the edits in reverse order to
+            // roll us back.
             var initialTextSnapshot = this.SubjectBuffer.CurrentSnapshot;
-            var initialVirtualCaretPosition = this.TextView.Caret.Position.VirtualBufferPosition;
 
             var initialCaretPosition = GetCaretPointInViewBuffer();
 
-            // When a character is typed it is *always* sent through to the editor.  This way the
-            // editor always represents what would have been typed had completion not been involved
-            // at this point.  After we send the character into the buffer we then decide what to do
-            // with the completion set.  If we decide to commit it then we will replace the
-            // appropriate span (which will include the character just sent to the buffer) with the
-            // appropriate insertion text *and* the character typed.  This way, after we commit, the
-            // editor has the insertion text of the selected item, and the character typed.  It
-            // also means that if we then undo that we'll see the text that would have been typed
-            // had no completion been active.
-
             // Note: while we're doing this, we don't want to hear about buffer changes (since we
             // know they're going to happen).  So we disconnect and reconnect to the event
             // afterwards.  That way we can hear about changes to the buffer that don't happen
@@ -88,8 +99,7 @@ void ICommandHandler<TypeCharCommandArgs>.ExecuteCommand(TypeCharCommandArgs arg
                     {
                         Trace.WriteLine("typechar was on seam and a commit char, cannot have a completion session.");
 
-                        this.CommitOnTypeChar(
-                            args.TypedChar, initialTextSnapshot, initialVirtualCaretPosition, nextHandler);
+                        this.CommitOnTypeChar(args.TypedChar, initialTextSnapshot, nextHandler);
                         return;
                     }
                     else if (_autoBraceCompletionChars.Contains(args.TypedChar) &&
@@ -100,8 +110,7 @@ void ICommandHandler<TypeCharCommandArgs>.ExecuteCommand(TypeCharCommandArgs arg
 
                         // I don't think there is any better way than this. if typed char is one of auto brace completion char,
                         // we don't do multiple buffer change check
-                        this.CommitOnTypeChar(
-                            args.TypedChar, initialTextSnapshot, initialVirtualCaretPosition, nextHandler);
+                        this.CommitOnTypeChar(args.TypedChar, initialTextSnapshot, nextHandler);
                         return;
                     }
                     else
@@ -221,8 +230,7 @@ void ICommandHandler<TypeCharCommandArgs>.ExecuteCommand(TypeCharCommandArgs arg
 
                         // Known to be a commit character for the currently selected item.  So just
                         // commit the session.
-                        this.CommitOnTypeChar(
-                            args.TypedChar, initialTextSnapshot, initialVirtualCaretPosition, nextHandler);
+                        this.CommitOnTypeChar(args.TypedChar, initialTextSnapshot, nextHandler);
                     }
                     else
                     {
@@ -438,9 +446,7 @@ private string GetTextTypedSoFar(Model model, CompletionItem selectedItem)
         }
 
         private void CommitOnTypeChar(
-            char ch, ITextSnapshot initialTextSnapshot, 
-            VirtualSnapshotPoint initialCaretPointInView,
-            Action nextHandler)
+            char ch, ITextSnapshot initialTextSnapshot, Action nextHandler)
         {
             AssertIsForeground();
 
@@ -452,8 +458,9 @@ private string GetTextTypedSoFar(Model model, CompletionItem selectedItem)
             // was commit character if we had a selected item.
             Contract.ThrowIfNull(model);
 
-            this.Commit(model.SelectedItem, model, ch, 
-                initialTextSnapshot, initialCaretPointInView, nextHandler);
+            this.Commit(
+                model.SelectedItem, model, ch,
+                initialTextSnapshot, nextHandler);
         }
     }
 }
\ No newline at end of file