[ObjectValue] Some more refactoring to invert controller and view communication

5 files changed, 245 insertions, 82 deletions

diff --git a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/DebugValueWindow.cs b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/DebugValueWindow.cs
index 17526e2093..28f78abd1b 100644
--- a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/DebugValueWindow.cs
+++ b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/DebugValueWindow.cs
@@ -101,11 +101,14 @@ namespace MonoDevelop.Debugger
 				controller.PinnedWatchFile = pinnedWatchFileName;
 
 				controller.PinStatusChanged += OnPinStatusChanged;
-				controller.StartEditing += OnStartEditing;
-				controller.EndEditing += OnEndEditing;
 
 				treeView = (TreeView) controller.GetControl (headersVisible: false, allowPinning: true, compactView: true);
 
+				if (treeView is IObjectValueTreeView ovtv) {
+					ovtv.StartEditing += OnStartEditing;
+					ovtv.EndEditing += OnEndEditing;
+				}
+
 				controller.AddValue (value);
 			} else {
 				objValueTreeView = new ObjectValueTreeView ();
@@ -168,8 +171,11 @@ namespace MonoDevelop.Debugger
 		{
 			if (UseNewTreeView) {
 				controller.PinStatusChanged -= OnPinStatusChanged;
-				controller.StartEditing -= OnStartEditing;
-				controller.EndEditing -= OnEndEditing;
+
+				if (treeView is IObjectValueTreeView ovtv) {
+					ovtv.StartEditing -= OnStartEditing;
+					ovtv.EndEditing -= OnEndEditing;
+				}
 			} else {
 				objValueTreeView.PinStatusChanged -= OnPinStatusChanged;
 				objValueTreeView.StartEditing -= OnStartEditing;
diff --git a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/Gtk/GtkObjectValueTreeView.cs b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/Gtk/GtkObjectValueTreeView.cs
index 99b14a92b3..14d005b407 100644
--- a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/Gtk/GtkObjectValueTreeView.cs
+++ b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/Gtk/GtkObjectValueTreeView.cs
@@ -279,10 +279,10 @@ namespace MonoDevelop.Debugger
 			PreviewWindowManager.WindowClosed += HandlePreviewWindowClosed;
 			ScrollAdjustmentsSet += HandleScrollAdjustmentsSet;
 
-			expanderSize = (int) StyleGetProperty ("expander-size") + 4; //+4 is hardcoded in gtk.c code
-			horizontal_separator = (int) StyleGetProperty ("horizontal-separator");
-			grid_line_width = (int) StyleGetProperty ("grid-line-width");
-			focus_line_width = (int) StyleGetProperty ("focus-line-width") * 2; //we just use *2 version in GetMaxWidth
+			expanderSize = (int)StyleGetProperty ("expander-size") + 4; //+4 is hardcoded in gtk.c code
+			horizontal_separator = (int)StyleGetProperty ("horizontal-separator");
+			grid_line_width = (int)StyleGetProperty ("grid-line-width");
+			focus_line_width = (int)StyleGetProperty ("focus-line-width") * 2; //we just use *2 version in GetMaxWidth
 
 			AdjustColumnSizes ();
 		}
@@ -322,6 +322,51 @@ namespace MonoDevelop.Debugger
 			}
 		}
 
+		/// <summary>
+		/// Triggered when the view requests a node to fetch more of it's children
+		/// </summary>
+		public event EventHandler<ObjectValueNodeEventArgs> NodeLoadMoreChildren;
+
+		/// <summary>
+		/// Triggered when the view needs the node to be refreshed
+		/// </summary>
+		public event EventHandler<ObjectValueNodeEventArgs> NodeRefresh;
+
+		/// <summary>
+		/// Triggered when the view needs to know if the node can be edited
+		/// </summary>
+		public event EventHandler<ObjectValueCanEditEventArgs> NodeGetCanEdit;
+
+		/// <summary>
+		/// Triggered when the node's value has been edited by the user
+		/// </summary>
+		public event EventHandler<ObjectValueEditEventArgs> NodeEditValue;
+
+		/// <summary>
+		/// Triggered when the user removes a node (an expression)
+		/// </summary>
+		public event EventHandler<ObjectValueNodeEventArgs> NodeRemoved;
+
+		/// <summary>
+		/// Triggered when an expression is added to the tree by the user
+		/// </summary>
+		public event EventHandler<ObjectValueExpressionEventArgs> ExpressionAdded;
+
+		/// <summary>
+		/// Triggered when an expression is edited by the user
+		/// </summary>
+		public event EventHandler<ObjectValueExpressionEventArgs> ExpressionEdited;
+
+		/// <summary>
+		/// Triggered when the user starts editing a node
+		/// </summary>
+		public event EventHandler StartEditing;
+
+		/// <summary>
+		/// Triggered when the user stops editing a node
+		/// </summary>
+		public event EventHandler EndEditing;
+
 		protected override void OnDestroyed ()
 		{
 			CompletionWindowManager.WindowClosed -= HandleCompletionWindowClosed;
@@ -391,7 +436,7 @@ namespace MonoDevelop.Debugger
 				if (string.IsNullOrWhiteSpace (expression))
 					continue;
 
-				controller.AddExpression (expression.Trim ());
+				ExpressionAdded?.Invoke (this, new ObjectValueExpressionEventArgs(null, expression.Trim ()));
 			}
 		}
 
@@ -654,15 +699,16 @@ namespace MonoDevelop.Debugger
 
 			if (node.IsEnumerable) {
 				if (node is ShowMoreValuesObjectValueNode moreNode) {
-					controller.FetchMoreChildrenAsync (moreNode.EnumerableNode).Ignore ();
+					NodeLoadMoreChildren?.Invoke (this, new ObjectValueNodeEventArgs (moreNode.EnumerableNode));
 				} else {
 					// use ExpandRow to expand so we see the loading message, expanding the node will trigger a fetch of the children
 					var treePath = GetTreePathForNode (node);
 					ExpandRow (treePath, false);
 				}
 			} else {
-				// this is likely to support IsImplicitNotSupported 
-				controller.RefreshNode (node);
+				// this is likely to support IsImplicitNotSupported
+				NodeRefresh?.Invoke (this, new ObjectValueNodeEventArgs (node));
+
 				// update the tree
 				if (store.IterParent (out TreeIter parentIter, it)) {
 					SetValues (parentIter, it, null, node);
@@ -772,7 +818,7 @@ namespace MonoDevelop.Debugger
 			if (updateJustValue)
 				return;
 
-			bool canEdit = controller.CanEditObject (val);
+			bool canEdit = GetCanEditNode (val);
 			string icon = ObjectValueTreeViewController.GetIcon (val.Flags);
 
 			store.SetValue (it, NameColumn, name);
@@ -817,6 +863,13 @@ namespace MonoDevelop.Debugger
 			}
 		}
 
+		bool GetCanEditNode(ObjectValueNode node)
+		{
+			var args = new ObjectValueCanEditEventArgs (node);
+			NodeGetCanEdit?.Invoke (this, args);
+			return args.CanEdit;
+		}
+
 		protected override bool OnTestExpandRow (TreeIter iter, TreePath path)
 		{
 			if (!restoringState) {
@@ -903,10 +956,11 @@ namespace MonoDevelop.Debugger
 			var node = GetNodeAtIter (iter);
 
 			if (node == null) {
-				if (args.NewText.Length > 0)
-					controller.AddExpression (args.NewText);
+				if (args.NewText.Length > 0) {
+					ExpressionAdded?.Invoke (this, new ObjectValueExpressionEventArgs (null, args.NewText));
+				}
 			} else {
-				controller.EditExpression (node, args.NewText);
+				ExpressionEdited?.Invoke (this, new ObjectValueExpressionEventArgs (null, args.NewText));
 			}
 		}
 
@@ -947,7 +1001,9 @@ namespace MonoDevelop.Debugger
 
 			// get the node that we just edited
 			var val = GetNodeAtIter (iter);
-			if (controller.EditNodeValue (val, args.NewText)) {
+			var editArgs = new ObjectValueEditEventArgs (val, args.NewText);
+			NodeEditValue?.Invoke (this, editArgs);
+			if (editArgs.Edited) {
 				// update the store
 				//store.SetValue (it, ValueColumn, val.GetDisplayValue());
 				SetValues (TreeIter.Zero, iter, null, val);
@@ -966,7 +1022,7 @@ namespace MonoDevelop.Debugger
 			editEntry.KeyPressEvent += OnEditKeyPress;
 			editEntry.KeyReleaseEvent += OnEditKeyRelease;
 
-			controller.OnStartEditing ();
+			StartEditing?.Invoke(this, EventArgs.Empty);
 		}
 
 		void OnEndEditing ()
@@ -978,7 +1034,7 @@ namespace MonoDevelop.Debugger
 			CompletionWindowManager.HideWindow ();
 			currentCompletionData = null;
 
-			controller.OnEndEditing ();
+			EndEditing?.Invoke (this, EventArgs.Empty);
 		}
 
 		void OnEditKeyRelease (object sender, EventArgs e)
@@ -1201,8 +1257,8 @@ namespace MonoDevelop.Debugger
 						continue;
 
 					var node = GetNodeAtIter (iter);
-					if (controller.RemoveValue (node))
-						changed = true;
+					NodeRemoved?.Invoke (this, new ObjectValueNodeEventArgs (node));
+					changed = true;
 
 					//val = GetDebuggerObjectValueAtIter (iter);
 					//expression = GetFullExpression (iter);
@@ -1451,8 +1507,9 @@ namespace MonoDevelop.Debugger
 				nodesToDelete.Add (node);
 			}
 
-			foreach (var node in nodesToDelete) 
-				controller.RemoveValue (node);
+			foreach (var node in nodesToDelete) {
+				NodeRemoved?.Invoke (this, new ObjectValueNodeEventArgs (node));
+			}
 		}
 
 		[CommandUpdateHandler (EditCommands.Delete)]
diff --git a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/IObjectValueTreeView.cs b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/IObjectValueTreeView.cs
index 8411f73c4b..5625bee3fe 100644
--- a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/IObjectValueTreeView.cs
+++ b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/IObjectValueTreeView.cs
@@ -34,17 +34,17 @@ namespace MonoDevelop.Debugger
 	public interface IObjectValueTreeView
 	{
 		/// <summary>
-		/// Gets a value indicating whether the user should be able to edit values in the tree
+		/// Gets or sets a value indicating whether the user should be able to edit values in the tree
 		/// </summary>
 		bool AllowEditing { get; set; }
 
 		/// <summary>
-		/// Gets a value indicating whether or not the user should be able to expand nodes in the tree
+		/// Gets or sets a value indicating whether or not the user should be able to expand nodes in the tree
 		/// </summary>
 		bool AllowExpanding { get; set; }
 
 		/// <summary>
-		/// Gets a value indicating whether the user should be able to add watch expressions to the tree
+		/// Gets or sets a value indicating whether the user should be able to add watch expressions to the tree
 		/// </summary>
 		bool AllowWatchExpressions { get; set; }
 
@@ -66,11 +66,59 @@ namespace MonoDevelop.Debugger
 		/// </summary>
 		void LoadEvaluatedNode (ObjectValueNode node, ObjectValueNode [] replacementNodes);
 
-
+		/// <summary>
+		/// Triggered when the view requests a node to fetch more of it's children
+		/// </summary>
+		event EventHandler<ObjectValueNodeEventArgs> NodeLoadMoreChildren;
 
 		event EventHandler<ObjectValueNodeEventArgs> NodeExpanded;
 		event EventHandler<ObjectValueNodeEventArgs> NodeCollapsed;
 
+		/// <summary>
+		/// Triggered when the view needs the node to be refreshed
+		/// </summary>
+		event EventHandler<ObjectValueNodeEventArgs> NodeRefresh;
+
+		/// <summary>
+		/// Triggered when the view needs to know if the node can be edited
+		/// </summary>
+		event EventHandler<ObjectValueCanEditEventArgs> NodeGetCanEdit;
+
+		/// <summary>
+		/// Triggered when the node's value has been edited by the user
+		/// </summary>
+		event EventHandler<ObjectValueEditEventArgs> NodeEditValue;
+
+		/// <summary>
+		/// Triggered when the user removes a node (an expression)
+		/// </summary>
+		event EventHandler<ObjectValueNodeEventArgs> NodeRemoved;
+
+		event EventHandler<ObjectValueDisplayEventArgs> NodeGetDisplayText;
+
+		/// <summary>
+		/// Triggered when an expression is added to the tree by the user
+		/// </summary>
+		event EventHandler<ObjectValueExpressionEventArgs> ExpressionAdded;
+
+		/// <summary>
+		/// Triggered when an expression is edited by the user
+		/// </summary>
+		event EventHandler<ObjectValueExpressionEventArgs> ExpressionEdited;
+
+
 		void OnNodeExpanded (ObjectValueNode node);
+
+
+
+		/// <summary>
+		/// Triggered when the user starts editing a node
+		/// </summary>
+		event EventHandler StartEditing;
+
+		/// <summary>
+		/// Triggered when the user stops editing a node
+		/// </summary>
+		event EventHandler EndEditing;
 	}
 }
diff --git a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueNodeEventArgs.cs b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueNodeEventArgs.cs
index f230832654..e2982e033e 100644
--- a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueNodeEventArgs.cs
+++ b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueNodeEventArgs.cs
@@ -40,38 +40,61 @@ namespace MonoDevelop.Debugger
 		}
 	}
 
-	public sealed class ObjectValueNodeEvaluationCompletedEventArgs : ObjectValueNodeEventArgs
+	public sealed class ObjectValueCanEditEventArgs : ObjectValueNodeEventArgs
 	{
-		public ObjectValueNodeEvaluationCompletedEventArgs (ObjectValueNode node, ObjectValueNode [] replacementNodes) : base (node)
+		public ObjectValueCanEditEventArgs (ObjectValueNode node) : base (node)
 		{
-			ReplacementNodes = replacementNodes;
 		}
 
-		/// <summary>
-		/// Gets an array of nodes that should be used to replace the node that finished evaluating.
-		/// Some sets of values, like local variables, frame locals and the like are fetched asynchronously
-		/// and may take some time to fetch. In this case, a single object is returned that is a place holder
-		/// for 0 or more values that should be expanded in the place of the evaluating node.
-		/// </summary>
-		public ObjectValueNode[] ReplacementNodes { get; }
+		public bool CanEdit {
+			get; set;
+		}
+	}
+
+	public sealed class ObjectValueEditEventArgs : ObjectValueNodeEventArgs
+	{
+		public ObjectValueEditEventArgs (ObjectValueNode node, string newValue) : base (node)
+		{
+			NewValue = newValue;
+		}
+
+		public string NewValue {
+			get; private set;
+		}
+
+		public bool Edited {
+			get; set;
+		}
 	}
 
-	public sealed class ObjectValueNodeChildrenChangedEventArgs : ObjectValueNodeEventArgs
+	public sealed class ObjectValueDisplayEventArgs : ObjectValueNodeEventArgs
 	{
-		public ObjectValueNodeChildrenChangedEventArgs (ObjectValueNode node, int index, int count) : base (node)
+		public ObjectValueDisplayEventArgs (ObjectValueNode node) : base (node)
 		{
-			Index = index;
-			Count = count;
 		}
 
-		/// <summary>
-		/// Gets the count of child nodes that were loaded
-		/// </summary>
-		public int Count { get; }
+		public string DisplayValue {
+			get; private set;
+		}
+
+		public bool HasVisualisers {
+			get; set;
+		}
+
+		public bool HasChangedSinceLastCheckpoint {
+			get; set;
+		}
+	}
+
+	public sealed class ObjectValueExpressionEventArgs : ObjectValueNodeEventArgs
+	{
+		public ObjectValueExpressionEventArgs (ObjectValueNode node, string expression) : base(node)
+		{
+			Expression = expression;
+		}
 
-		/// <summary>
-		/// Gets the index of the first child that was loaded
-		/// </summary>
-		public int Index { get; }
+		public string Expression {
+			get; private set;
+		}
 	}
 }
diff --git a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueTreeViewController.cs b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueTreeViewController.cs
index 96695e5f83..1224a7b3cd 100644
--- a/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueTreeViewController.cs
+++ b/main/src/addins/MonoDevelop.Debugger/MonoDevelop.Debugger/ObjectValue/ObjectValueTreeViewController.cs
@@ -200,20 +200,6 @@ namespace MonoDevelop.Debugger
 			OnPinStatusChanged ();
 		}
 
-		public event EventHandler StartEditing;
-
-		internal void OnStartEditing ()
-		{
-			StartEditing?.Invoke (this, EventArgs.Empty);
-		}
-
-		public event EventHandler EndEditing;
-
-		internal void OnEndEditing ()
-		{
-			EndEditing?.Invoke (this, EventArgs.Empty);
-		}
-
 		public object GetControl (bool headersVisible = true, bool compactView = false, bool allowPinning = false, bool allowPopupMenu = true)
 		{
 			if (view == null) {
@@ -223,6 +209,13 @@ namespace MonoDevelop.Debugger
 
 				view.NodeExpanded += OnViewNodeExpanded;
 				view.NodeCollapsed += OnViewNodeCollapsed;
+				view.NodeLoadMoreChildren += OnViewNodeLoadMoreChildren;
+				view.ExpressionAdded += OnViewExpressionAdded;
+				view.ExpressionEdited += OnViewExpressionEdited;
+				view.NodeRefresh += OnViewNodeRefresh;
+				view.NodeGetCanEdit += OnViewNodeCanEdit;
+				view.NodeEditValue += OnViewNodeEditValue;
+				view.NodeRemoved += OnViewNodeRemoved;
 			}
 
 			return view;
@@ -284,12 +277,10 @@ namespace MonoDevelop.Debugger
 			}).Ignore ();
 		}
 
-		public bool RemoveValue (ObjectValueNode node)
+		void RemoveValue (ObjectValueNode node)
 		{
 			UnregisterNode (node);
 			OnEvaluationCompleted (node, new ObjectValueNode [0]);
-
-			return true;
 		}
 
 		/// <summary>
@@ -414,7 +405,7 @@ namespace MonoDevelop.Debugger
 		/// <summary>
 		/// Returns true if the node can be edited
 		/// </summary>
-		public bool CanEditObject (ObjectValueNode node)
+		bool CanEditObject (ObjectValueNode node)
 		{
 			if (AllowEditing) {
 				// TODO: clean up
@@ -434,7 +425,7 @@ namespace MonoDevelop.Debugger
 		/// Edits the value of the node and returns a value indicating whether the node's value changed from
 		/// when the node was initially loaded from the debugger
 		/// </summary>
-		public bool EditNodeValue (ObjectValueNode node, string newValue)
+		bool EditNodeValue (ObjectValueNode node, string newValue)
 		{
 			if (node == null || !AllowEditing)
 				return false;
@@ -542,6 +533,57 @@ namespace MonoDevelop.Debugger
 			}
 		}
 
+		#region View event handlers
+		void OnViewNodeExpanded (object sender, ObjectValueNodeEventArgs e)
+		{
+			ExpandNodeAsync (e.Node).Ignore ();
+		}
+
+		/// <summary>
+		/// Marks a node as not expanded
+		/// </summary>
+		void OnViewNodeCollapsed (object sender, ObjectValueNodeEventArgs e)
+		{
+			e.Node.IsExpanded = false;
+		}
+
+		void OnViewNodeLoadMoreChildren (object sender, ObjectValueNodeEventArgs e)
+		{
+			FetchMoreChildrenAsync (e.Node).Ignore ();
+		}
+
+		void OnViewExpressionAdded (object sender, ObjectValueExpressionEventArgs e)
+		{
+			AddExpression (e.Expression);
+		}
+
+		void OnViewExpressionEdited (object sender, ObjectValueExpressionEventArgs e)
+		{
+			EditExpression (e.Node, e.Expression);
+		}
+
+		void OnViewNodeRefresh (object sender, ObjectValueNodeEventArgs e)
+		{
+			RefreshNode (e.Node);
+		}
+
+		void OnViewNodeCanEdit (object sender, ObjectValueCanEditEventArgs e)
+		{
+			e.CanEdit = CanEditObject (e.Node);
+		}
+
+		void OnViewNodeEditValue (object sender, ObjectValueEditEventArgs e)
+		{
+			e.Edited = EditNodeValue (e.Node, e.NewValue);
+		}
+
+		void OnViewNodeRemoved (object sender, ObjectValueNodeEventArgs e)
+		{
+			RemoveValue (e.Node);
+		}
+
+		#endregion
+
 		#region Fetching and loading children
 		/// <summary>
 		/// Marks a node as expanded and fetches children for the node if they have not been already fetched
@@ -574,20 +616,7 @@ namespace MonoDevelop.Debugger
 			});
 		}
 
-		void OnViewNodeExpanded (object sender, ObjectValueNodeEventArgs e)
-		{
-			ExpandNodeAsync (e.Node).Ignore ();
-		}
-
-		/// <summary>
-		/// Marks a node as not expanded
-		/// </summary>
-		void OnViewNodeCollapsed (object sender, ObjectValueNodeEventArgs e)
-		{
-			e.Node.IsExpanded = false;
-		}
-
-		public async Task<int> FetchMoreChildrenAsync (ObjectValueNode node)
+		async Task<int> FetchMoreChildrenAsync (ObjectValueNode node)
 		{
 			if (node.ChildrenLoaded) {
 				return 0;