Merge pull request #404 from xamarin/fix-795831

[Version Control] Committing behaviour with unsaved files

3 files changed, 144 insertions, 75 deletions

diff --git a/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Dialogs/CommitDialog.cs b/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Dialogs/CommitDialog.cs
index d1503e3819..c639251548 100644
--- a/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Dialogs/CommitDialog.cs
+++ b/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Dialogs/CommitDialog.cs
@@ -30,13 +30,9 @@
 // THE SOFTWARE.
 
 using System;
-using System.Collections;
 using Gtk;
 using MonoDevelop.Core;
- 
-using MonoDevelop.Ide.Gui;
 using Mono.Addins;
-using MonoDevelop.Ide;
 using MonoDevelop.Projects;
 using MonoDevelop.Components;
 using System.Collections.Generic;
@@ -47,7 +43,7 @@ namespace MonoDevelop.VersionControl.Dialogs
 	partial class CommitDialog : Gtk.Dialog
 	{
 		ListStore store;
-		List<FilePath> selected = new List<FilePath> ();
+		HashSet<FilePath> selected = new HashSet<FilePath> ();
 		List<CommitDialogExtension> extensions = new List<CommitDialogExtension> ();
 		ChangeSet changeSet;
 		string oldMessage;
@@ -112,24 +108,7 @@ namespace MonoDevelop.VersionControl.Dialogs
 			}
 			HandleAllowCommitChanged (null, null);
 
-			foreach (ChangeSetItem info in changeSet.Items) {
-				Xwt.Drawing.Image statusicon = VersionControlService.LoadIconForStatus (info.Status);
-				string lstatus = VersionControlService.GetStatusLabel (info.Status);
-				string localpath;
-
-				if (info.IsDirectory)
-					localpath = (!info.LocalPath.IsChildPathOf (changeSet.BaseLocalPath)?
-									".":
-									(string) info.LocalPath.ToRelative (changeSet.BaseLocalPath));
-				else
-					localpath = System.IO.Path.GetFileName((string) info.LocalPath);
-
-				if (localpath.Length > 0 && localpath[0] == System.IO.Path.DirectorySeparatorChar) localpath = localpath.Substring(1);
-				if (localpath == "") { localpath = "."; } // not sure if this happens
-				
-				store.AppendValues (statusicon, lstatus, localpath, true, info);
-				selected.Add (info.LocalPath);
-			}
+			LoadChangeset (changeSet.Items);
 
 			if (string.IsNullOrEmpty (changeSet.GlobalComment)) {
 				AuthorInformation aInfo;
@@ -150,6 +129,58 @@ namespace MonoDevelop.VersionControl.Dialogs
 			textview.Buffer.MarkSet += OnMarkSet;
 
 			SetResponseSensitive (ResponseType.Ok, responseSensitive);
+
+			VersionControlService.FileStatusChanged += OnFileStatusChanged;
+		}
+
+		void OnFileStatusChanged (object sender, FileUpdateEventArgs args)
+		{
+			foreach (FileUpdateEventInfo f in args) {
+				OnFileStatusChanged (f);
+			}
+		}
+
+		void OnFileStatusChanged (FileUpdateEventInfo args)
+		{
+			VersionInfo newInfo = null;
+			try {
+				// Reuse remote status from old version info
+				newInfo = changeSet.Repository.GetVersionInfo (args.FilePath);
+			} catch (Exception ex) {
+				LoggingService.LogError (ex.ToString ());
+			}
+			AddFile (newInfo);
+		}
+
+		void AddFile (VersionInfo vinfo)
+		{
+			if (vinfo != null && (vinfo.HasLocalChanges || vinfo.HasRemoteChanges)) {
+				changeSet.AddFile (vinfo.LocalPath);
+				bool added = selected.Add (vinfo.LocalPath);
+				if (added)
+					AppendFileInfo (vinfo);
+			}
+		}
+
+		TreeIter AppendFileInfo (VersionInfo info)
+		{
+			Xwt.Drawing.Image statusicon = VersionControlService.LoadIconForStatus (info.Status);
+			string lstatus = VersionControlService.GetStatusLabel (info.Status);
+			string localpath;
+
+			if (info.IsDirectory)
+				localpath = (!info.LocalPath.IsChildPathOf (changeSet.BaseLocalPath) ?
+								"." :
+								(string)info.LocalPath.ToRelative (changeSet.BaseLocalPath));
+			else
+				localpath = System.IO.Path.GetFileName ((string)info.LocalPath);
+
+			if (localpath.Length > 0 && localpath [0] == System.IO.Path.DirectorySeparatorChar) localpath = localpath.Substring (1);
+			if (localpath == "") { localpath = "."; } 
+
+			TreeIter it = store.AppendValues (statusicon, lstatus, localpath, true, info);
+
+			return it;
 		}
 
 		void HandleAllowCommitChanged (object sender, EventArgs e)
@@ -159,7 +190,7 @@ namespace MonoDevelop.VersionControl.Dialogs
 				allowCommit &= ext.AllowCommit;
 			SetResponseSensitive (Gtk.ResponseType.Ok, allowCommit);
 		}
-		
+
 		protected override void OnResponse (Gtk.ResponseType type)
 		{
 			if (type != Gtk.ResponseType.Ok) {
@@ -172,6 +203,8 @@ namespace MonoDevelop.VersionControl.Dialogs
 
 		protected override void OnDestroyed ()
 		{
+			VersionControlService.FileStatusChanged -= OnFileStatusChanged;
+
 			foreach (var ob in extensions) {
 				var ext = ob as CommitDialogExtension;
 				if (ext != null)
@@ -180,60 +213,38 @@ namespace MonoDevelop.VersionControl.Dialogs
 			base.OnDestroyed ();
 		}
 
-		bool ButtonCommitClicked ()
+		void LoadChangeset (IEnumerable<ChangeSetItem> items)
 		{
-			// In case we have local unsaved files with changes, throw a dialog for the user.
-			System.Collections.Generic.List<Document> docList = new System.Collections.Generic.List<Document> ();
-			foreach (var item in IdeApp.Workbench.Documents) {
-				if (!item.IsDirty || !selected.Contains (item.FileName))
-					continue;
-				docList.Add (item);
-			}
+			fileList.Model = null;
+			store.Clear ();
 
-			if (docList.Count != 0) {
-				AlertButton response = MessageService.GenericAlert (
-					MonoDevelop.Ide.Gui.Stock.Question,
-					GettextCatalog.GetString ("You are trying to commit files which have unsaved changes."),
-					GettextCatalog.GetString ("Do you want to save the changes before committing?"),
-					new AlertButton[] {
-						AlertButton.Cancel,
-						new AlertButton (GettextCatalog.GetString ("Don't Save")),
-						AlertButton.Save
-					}
-				);
+			foreach (ChangeSetItem info in items) {
+				Xwt.Drawing.Image statusicon = VersionControlService.LoadIconForStatus (info.Status);
+				string lstatus = VersionControlService.GetStatusLabel (info.Status);
+				string localpath;
 
-				if (response == AlertButton.Cancel)
-					return false;
+				if (info.IsDirectory)
+					localpath = (!info.LocalPath.IsChildPathOf (changeSet.BaseLocalPath) ?
+									"." :
+									(string)info.LocalPath.ToRelative (changeSet.BaseLocalPath));
+				else
+					localpath = System.IO.Path.GetFileName ((string)info.LocalPath);
 
-				if (response == AlertButton.Save) {
-					// Go through all the items and save them.
-					foreach (var item in docList)
-						item.Save ();
-
-					// Check if save failed on any item and abort.
-					foreach (var item in docList)
-						if (item.IsDirty) {
-							MessageService.ShowMessage (GettextCatalog.GetString (
-								"Some files could not be saved. Commit operation aborted"));
-							return false;
-						}
-				}
+				if (localpath.Length > 0 && localpath [0] == System.IO.Path.DirectorySeparatorChar) localpath = localpath.Substring (1);
+				if (localpath == "") { localpath = "."; } // not sure if this happens
 
-				docList.Clear ();
+				store.AppendValues (statusicon, lstatus, localpath, true, info);
+				selected.Add (info.LocalPath);
 			}
 
-			// Update the change set
-			List<FilePath> todel = new List<FilePath> ();
-			foreach (ChangeSetItem it in changeSet.Items) {
-				if (!selected.Contains (it.LocalPath))
-					todel.Add (it.LocalPath);
-			}
-			foreach (string file in todel)
-				changeSet.RemoveFile (file);
+			fileList.Model = store;
+		}
+
+		bool ButtonCommitClicked ()
+		{
 			changeSet.GlobalComment = Message;
 			
 			// Perform the commit
-			
 			int n;
 			for (n=0; n<extensions.Count; n++) {
 				CommitDialogExtension ext = extensions [n];
@@ -250,7 +261,9 @@ namespace MonoDevelop.VersionControl.Dialogs
 						ext = extensions [m];
 						try {
 							ext.OnEndCommit (changeSet, false);
-						} catch {}
+						} catch (Exception ex) {
+							LoggingService.LogInternalError ("Commit operation failed.", ex);
+						}
 					}
 					return false;
 				}
diff --git a/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Views/StatusView.cs b/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Views/StatusView.cs
index ffdfe5d951..8f9a2d1a48 100644
--- a/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Views/StatusView.cs
+++ b/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl.Views/StatusView.cs
@@ -774,7 +774,7 @@ namespace MonoDevelop.VersionControl.Views
 			StartUpdate ();
 		}
 
-		void OnCommitClicked (object src, EventArgs args)
+		async void OnCommitClicked (object src, EventArgs args)
 		{
 			// Nothing to commit
 			if (changeSet.IsEmpty)
@@ -789,7 +789,7 @@ namespace MonoDevelop.VersionControl.Views
 					return;
 			}
 
-			CommitCommand.Commit (vc, changeSet.Clone ());
+			await CommitCommand.CommitAsync (vc, changeSet.Clone ());
 		}
 
 
diff --git a/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl/CommitCommand.cs b/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl/CommitCommand.cs
index b26ac01403..6114c0d8a8 100644
--- a/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl/CommitCommand.cs
+++ b/main/src/addins/VersionControl/MonoDevelop.VersionControl/MonoDevelop.VersionControl/CommitCommand.cs
@@ -1,19 +1,23 @@
 
 using System;
-using System.Collections;
-using MonoDevelop.VersionControl.Dialogs;
 using MonoDevelop.Core;
+using MonoDevelop.Ide.Gui;
 using MonoDevelop.Ide;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using MonoDevelop.VersionControl.Dialogs;
+using System.Linq;
 
 namespace MonoDevelop.VersionControl
 {
 	class CommitCommand
 	{
-		public static void Commit (Repository vc, ChangeSet changeSet)
+		public static async Task CommitAsync (Repository vc, ChangeSet changeSet)
 		{
 			try {
 				VersionControlService.NotifyPrepareCommit (vc, changeSet);
-
+				if (!await VerifyUnsavedChangesAsync (changeSet))
+					return;
 				CommitDialog dlg = new CommitDialog (changeSet);
 				try {
 					if (MessageService.RunCustomDialog (dlg) == (int) Gtk.ResponseType.Ok) {
@@ -33,6 +37,58 @@ namespace MonoDevelop.VersionControl
 			}
 		}
 
+		static async Task<bool> VerifyUnsavedChangesAsync (ChangeSet changeSet)
+		{
+			bool allowCommit = true;
+			// In case we have local unsaved files with changes, ask the user to save them.
+			List<Document> docList = new List<Document> ();
+			foreach (var item in IdeApp.Workbench.Documents) {
+				if (item.IsDirty)
+					docList.Add (item);
+			}
+
+			if (docList.Count != 0) {
+				AlertButton dontSaveButton = new AlertButton (GettextCatalog.GetString ("Don't Save"));
+				AlertButton response = MessageService.GenericAlert (
+					Stock.Question,
+					GettextCatalog.GetString ("You are trying to commit files which have unsaved changes."),
+					GettextCatalog.GetString ("Do you want to save the changes before committing?"),
+					new AlertButton [] {
+						AlertButton.Cancel,
+						dontSaveButton,
+						AlertButton.Save
+					}
+				);
+
+				if (response == AlertButton.Cancel) {
+					return false;
+				}
+
+				if (response == dontSaveButton) {
+					allowCommit = true;
+				}
+
+				if (response == AlertButton.Save) {
+					// Go through all the items and save them.
+					foreach (var item in docList) {
+						await item.Save ();
+						if (!changeSet.ContainsFile (item.FileName) && allowCommit)
+							allowCommit = false;
+					}
+
+					// Check if save failed on any item.
+					foreach (var item in docList)
+						if (item.IsDirty) {
+							MessageService.ShowMessage (GettextCatalog.GetString (
+								"Some files could not be saved."));
+							return false;
+						}
+				}
+			}
+
+			return allowCommit;
+		}
+
 		private class CommitWorker : VersionControlTask
 		{
 			Repository vc;