Merge pull request #317 from gui-cs/better-response-when-ref

When trying to delete a referenced view, give feedback about the error

Author: tznind

src/Operations/DeleteViewOperation.cs

@@ -13,6 +13,12 @@ public class DeleteViewOperation : Operation
     private readonly View[] from;
     private readonly Design[] originalSelection;
 
+    /// <summary>
+    /// Views which reference the <see cref="Design"/> being operated on and so would
+    /// crash or break if the view were deleted
+    /// </summary>
+    public Design[] PreventDeleting { get; set; }
+
     /// <summary>
     /// Initializes a new instance of the <see cref="DeleteViewOperation"/> class.
     /// </summary>
@@ -34,7 +40,9 @@ public DeleteViewOperation(params Design[] delete)
 
             // there are view(s) that depend on us (e.g. for positioning)
             // that are not also being deleted themselves
-            if (design.GetDependantDesigns().Any(dep => !delete.Contains(dep)))
+            PreventDeleting = design.GetDependantDesigns().Where(dep => !delete.Contains(dep)).ToArray();
+            
+            if (PreventDeleting.Length > 0)
             {
                 // Prevent deleting - so there are no orphan references from existing Views e.g. Pos.Right(thingIJustDeleted);
                 this.IsImpossible = true;

src/Operations/DragOperation.cs

@@ -14,6 +14,13 @@ public partial class DragOperation : Operation
 
     private View? dropInto;
 
+
+    /// <summary>
+    /// Views which reference the <see cref="Design"/> being operated on and so would
+    /// crash or break if the view were moved to another container (see <see cref="DropInto"/>)
+    /// </summary>
+    public Design[] PreventDrag { get; set; }
+
     /// <summary>
     /// Initializes a new instance of the <see cref="DragOperation"/> class.
     /// Begins a drag operation in which <paramref name="beingDragged"/> is moved.
@@ -136,9 +143,17 @@ public View? DropInto
             }
 
             this.dropInto = value;
+
+
+            IsImpossible = dropInto != null && IsChangingContainer() && this.mementos.Any(HasDependantsThatAreNotAlsoPartOfDrag);
         }
     }
 
+    private bool IsChangingContainer()
+    {
+        return mementos.Any(m => m.OriginalSuperView != DropInto);
+    }
+
     /// <summary>
     /// Moves all dragged views back to original positions.
     /// </summary>
@@ -173,6 +188,18 @@ public void ContinueDrag(Point dest)
         }
     }
 
+    private bool HasDependantsThatAreNotAlsoPartOfDrag(DragMemento arg)
+    {
+        // there are view(s) that depend on us (e.g. for positioning)
+        // that are not also being deleted themselves
+
+        var alsoBeingDragged = new HashSet<Design>(mementos.Select(m => m.Design));
+        PreventDrag = arg.Design.GetDependantDesigns().Where(dep => !alsoBeingDragged.Contains(dep)).ToArray();
+
+        return PreventDrag.Any();
+    }
+
+
     /// <inheritdoc/>
     protected override bool DoImpl()
     {
@@ -260,4 +287,13 @@ private void ContinueDrag(DragMemento mem, Point dest)
 
         this.DestinationY = dest.Y;
     }
+
+    /// <summary>
+    /// Restores all mementos to original locations
+    /// </summary>
+    /// <exception cref="NotImplementedException"></exception>
+    public void Abandon()
+    {
+        UndoImpl();
+    }
 }

src/UI/Editor.cs

@@ -26,7 +26,7 @@ namespace TerminalGuiDesigner.UI;
 /// application.  Hooks key and mouse events and mounts as a sub-view whatever file
 /// the user opens.
 /// </summary>
-public class Editor : Toplevel
+public class Editor : Toplevel, IErrorReporter
 {
     private KeyMap keyMap;
     private readonly KeyboardManager keyboardManager;
@@ -61,6 +61,11 @@ public class Editor : Toplevel
     /// </summary>
     public static bool Quiet = false;
 
+    /// <summary>
+    /// Start your message with this if you want it to be visually highlighted as bad.
+    /// </summary>
+    public const string Error = "Error";
+
     /// <summary>
     /// Initializes a new instance of the <see cref="Editor"/> class.
     /// </summary>
@@ -77,7 +82,10 @@ public Editor()
         LoadKeyMap();
 
         this.keyboardManager = new KeyboardManager(this.keyMap);
-        this.mouseManager = new MouseManager();
+        this.mouseManager = new MouseManager()
+        {
+            ErrorReporter = this
+        };
         this.Closing += this.Editor_Closing;
 
         this.BuildRootMenu();
@@ -293,6 +301,13 @@ protected override void OnDrawComplete(DrawContext? context)
             // and have a designable view focused
             if (toDisplay != null)
             {
+                var before = GetCurrentAttribute();
+
+                if (toDisplay.StartsWith(Error))
+                {
+                    SetAttribute(new Attribute(Color.Red, Color.Black));
+                }
+
                 // write its name in the lower right
                 int y = this.GetContentSize().Height - 1;
                 int right = bounds.Width - 1;
@@ -303,6 +318,8 @@ protected override void OnDrawComplete(DrawContext? context)
                 {
                     this.AddRune(right - len + i, y, runes[i]);
                 }
+
+                SetAttribute(before);
             }
         }
 
@@ -1151,10 +1168,31 @@ private void Delete()
         if (SelectionManager.Instance.Selected.Any())
         {
             var cmd = new DeleteViewOperation(SelectionManager.Instance.Selected.ToArray());
+            
+            
+            if (cmd.IsImpossible && cmd.PreventDeleting.Any())
+            {
+                ShowErrorThatViewIsUsedByOthers(cmd.PreventDeleting);
+                return;
+            }
+
             OperationManager.Instance.Do(cmd);
         }
     }
 
+    public void ShowErrorThatViewIsUsedByOthers(Design[] usedBy)
+    {
+        if (usedBy.Length == 1)
+        {
+            flashMessage = $"{Error}, view referenced by " + usedBy[0].FieldName;
+        }
+        else
+        {
+            flashMessage = $"{Error}, view referenced by {usedBy.Length} views";
+        }
+        this.SetNeedsDraw();
+    }
+
     private void DoForSelectedViews(Func<Design, Operation> operationFunc, bool allowOnRoot = false)
     {
         if (this.viewBeingEdited == null)
@@ -1469,4 +1507,4 @@ private void ShowEditProperties(Design d)
         var edit = new EditDialog(d);
         Application.Run(edit, this.ErrorHandler);
     }
-}
+}

src/UI/IErrorReporter.cs

@@ -0,0 +1,6 @@
+namespace TerminalGuiDesigner.UI;
+
+public interface IErrorReporter
+{
+    public void ShowErrorThatViewIsUsedByOthers(Design[] usedBy);
+}

src/UI/MouseManager.cs

@@ -33,6 +33,12 @@ public class MouseManager
     /// </summary>
     public Rectangle? SelectionBox => RectExtensions.FromBetweenPoints(this.selectionStart, this.selectionEnd);
 
+    /// <summary>
+    /// Set to report impossible commands as they are executed i.e. to give info
+    /// to user about why operation failed etc.
+    /// </summary>
+    public IErrorReporter? ErrorReporter;
+
     /// <summary>
     /// Responds to <see cref="Application.MouseEvent"/>(by changing a 'drag a box' selection area
     /// or starting a resize etc).
@@ -198,6 +204,13 @@ public void HandleMouse(MouseEventArgs m, Design viewBeingEdited)
                     // we are dragging into a new container
                     this.dragOperation.DropInto = into.View;
 
+                   
+                    if (this.dragOperation.IsImpossible && this.dragOperation.PreventDrag.Any())
+                    {
+                        ErrorReporter?.ShowErrorThatViewIsUsedByOthers(this.dragOperation.PreventDrag);
+                        this.dragOperation.Abandon();
+                    }
+
                     // end drag
                     OperationManager.Instance.Do(this.dragOperation);
                     this.dragOperation = null;

tests/Operations/DeleteViewOperationTests.cs

@@ -103,4 +103,49 @@ public void TestDeleting_ClearsSelection(bool lockSelection)
         ClassicAssert.AreEqual(2, designOut.GetAllDesigns().Count());
         ClassicAssert.Contains(lbl1Design, SelectionManager.Instance.Selected.ToArray(), "Undoing a delete operation should restore the previous selection");
     }
+
+    [Test]
+    public void TestPreventDeleting_PopulatedWhenDependenciesExist()
+    {
+        var viewToCode = new ViewToCode();
+        var file = new FileInfo("TestPreventDeleting_PopulatedWhenDependenciesExist.cs");
+        var designOut = viewToCode.GenerateNewView(file, "YourNamespace", typeof(View));
+
+        var lbl1 = ViewFactory.Create<Label>();
+        var lbl2 = ViewFactory.Create<Label>();
+
+        // add 2 labels
+        new AddViewOperation(lbl1, designOut, "lbl1").Do();
+        new AddViewOperation(lbl2, designOut, "lbl2").Do();
+
+        // Add dependency: lbl2 depends on lbl1
+        lbl2.X = Pos.Right(lbl1) + 5;
+
+        var cmd = new DeleteViewOperation((Design)lbl1.Data);
+
+        ClassicAssert.IsTrue(cmd.IsImpossible, "Deleting lbl1 should be impossible because lbl2 depends on it");
+        ClassicAssert.AreEqual(1, cmd.PreventDeleting.Length, "PreventDeleting should contain exactly one dependent design");
+        ClassicAssert.AreSame(lbl2.Data, cmd.PreventDeleting[0], "PreventDeleting should contain lbl2 because it depends on lbl1");
+    }
+
+    [Test]
+    public void TestPreventDeleting_EmptyWhenNoDependencies()
+    {
+        var viewToCode = new ViewToCode();
+        var file = new FileInfo("TestPreventDeleting_EmptyWhenNoDependencies.cs");
+        var designOut = viewToCode.GenerateNewView(file, "YourNamespace", typeof(View));
+
+        var lbl1 = ViewFactory.Create<Label>();
+        var lbl2 = ViewFactory.Create<Label>();
+
+        // add 2 labels (no dependencies between them)
+        new AddViewOperation(lbl1, designOut, "lbl1").Do();
+        new AddViewOperation(lbl2, designOut, "lbl2").Do();
+
+        var cmd = new DeleteViewOperation((Design)lbl1.Data);
+
+        ClassicAssert.IsFalse(cmd.IsImpossible, "Deleting lbl1 should be possible because nothing depends on it");
+        ClassicAssert.IsEmpty(cmd.PreventDeleting, "PreventDeleting should be empty when no dependents exist");
+    }
+
 }