Include X and Y in ToString when it is specified in one of the constr…

…uctors (#1674).
dlemstra · Jul 12, 2024 · 0c8db17 · 0c8db17
1 parent d01d8ce
commit 0c8db17
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 13 deletions.
diff --git a/src/Magick.NET/Types/MagickGeometry.cs b/src/Magick.NET/Types/MagickGeometry.cs
@@ -11,26 +11,37 @@ namespace ImageMagick;
 /// </summary>
 public sealed partial class MagickGeometry : IMagickGeometry
 {
+    private readonly bool _includeXyInToString;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class.
     /// </summary>
     public MagickGeometry()
-        => Initialize(0, 0, 0, 0);
+    {
+        Initialize(0, 0, 0, 0);
+        _includeXyInToString = false;
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class using the specified width and height.
     /// </summary>
     /// <param name="widthAndHeight">The width and height.</param>
     public MagickGeometry(int widthAndHeight)
-        => Initialize(0, 0, widthAndHeight, widthAndHeight);
+    {
+        Initialize(0, 0, widthAndHeight, widthAndHeight);
+        _includeXyInToString = false;
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class using the specified width and height.
     /// </summary>
     /// <param name="width">The width.</param>
     /// <param name="height">The height.</param>
     public MagickGeometry(int width, int height)
-        => Initialize(0, 0, width, height);
+    {
+        Initialize(0, 0, width, height);
+        _includeXyInToString = false;
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class using the specified offsets, width and height.
@@ -40,15 +51,21 @@ public MagickGeometry(int width, int height)
     /// <param name="width">The width.</param>
     /// <param name="height">The height.</param>
     public MagickGeometry(int x, int y, int width, int height)
-        => Initialize(x, y, width, height);
+    {
+        Initialize(x, y, width, height);
+        _includeXyInToString = true;
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class using the specified width and height.
     /// </summary>
     /// <param name="percentageWidth">The percentage of the width.</param>
     /// <param name="percentageHeight">The percentage of the height.</param>
     public MagickGeometry(Percentage percentageWidth, Percentage percentageHeight)
-        => InitializeFromPercentage(0, 0, percentageWidth, percentageHeight);
+    {
+        InitializeFromPercentage(0, 0, percentageWidth, percentageHeight);
+        _includeXyInToString = false;
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class using the specified offsets, width and height.
@@ -58,7 +75,10 @@ public MagickGeometry(Percentage percentageWidth, Percentage percentageHeight)
     /// <param name="percentageWidth">The percentage of the width.</param>
     /// <param name="percentageHeight">The percentage of the height.</param>
     public MagickGeometry(int x, int y, Percentage percentageWidth, Percentage percentageHeight)
-        => InitializeFromPercentage(x, y, percentageWidth, percentageHeight);
+    {
+        InitializeFromPercentage(x, y, percentageWidth, percentageHeight);
+        _includeXyInToString = true;
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="MagickGeometry"/> class using the specified geometry.
@@ -76,6 +96,8 @@ public MagickGeometry(string value)
             Initialize(instance, flags);
         else
             InitializeFromAspectRation(instance, value);
+
+        _includeXyInToString = value.IndexOf("+") >= 0 || value.IndexOf("-") >= 0;
     }
 
     /// <summary>
@@ -325,15 +347,22 @@ public override string ToString()
 
         var result = string.Empty;
 
-        if (Width > 0)
-            result += Width;
+        if (Width == 0 && Height == 0)
+        {
+            result = "0x0";
+        }
+        else
+        {
+            if (Width > 0)
+                result += Width;
 
-        if (Height > 0)
-            result += "x" + Height;
-        else if (!IsPercentage)
-            result += "x";
+            if (Height > 0)
+                result += "x" + Height;
+            else if (!IsPercentage)
+                result += "x";
+        }
 
-        if (X != 0 || Y != 0)
+        if (X != 0 || Y != 0 || _includeXyInToString)
         {
             if (X >= 0)
                 result += '+';

diff --git a/tests/Magick.NET.Tests/Types/MagickGeometryTests/TheToStringMethod.cs b/tests/Magick.NET.Tests/Types/MagickGeometryTests/TheToStringMethod.cs
@@ -110,5 +110,41 @@ public void ShouldSetGreaterAndIsPercentage()
 
             Assert.Equal("50%>", geometry.ToString());
         }
+
+        [Fact]
+        public void ShouldIncludeZeroXYWhenSpecified()
+        {
+            var geometry = new MagickGeometry(0, 0, 5, 10);
+
+            Assert.Equal("5x10+0+0", geometry.ToString());
+        }
+
+        [Fact]
+        public void ShouldReturnTheCorrectStringWhenBothWidthAndHeightAreZero()
+        {
+            var geometry = new MagickGeometry(0, 0);
+
+            Assert.Equal("0x0", geometry.ToString());
+        }
+
+        [Fact]
+        public void ShouldCreateTheCorrectStringWhenAllValuesAreZero()
+        {
+            var geometry = new MagickGeometry(0, 0, 0, 0);
+
+            Assert.Equal("0x0+0+0", geometry.ToString());
+        }
+
+        [Theory]
+        [InlineData("0x0+0+0")]
+        [InlineData("0x0-0+0")]
+        [InlineData("0x0+0-0")]
+        [InlineData("0x0-0-0")]
+        public void ShouldIncludeZeroXYWhenSpecifiedInStringConstructor(string value)
+        {
+            var geometry = new MagickGeometry(value);
+
+            Assert.Equal("0x0+0+0", geometry.ToString());
+        }
     }
 }