Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions IconHelper.Test/ArgumentsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,54 @@ public void ValidateRejectsANegativeSizeAndPaddingTogether()
}

[TestMethod]
public void ValidateUsesIntegerDivisionForOddSizes()
public void ValidateAcceptsPaddingThatLeavesOnePixelOnAnOddSize()
{
// 33 / 2 == 16 under integer division, so 16 is rejected and 15 accepted.
// 16 pixels a side on a 33 pixel canvas leaves one pixel of content, which is what the
// pipeline's own clamp, (size - 1) / 2, allows. Integer division (33 / 2 == 16) used to
// reject it.
using TempDirectory temp = new();
Arguments rejected = ValidArguments(temp);
rejected.Size = 33;
rejected.Padding = 16;
rejected.Padding = 17;

Arguments accepted = ValidArguments(temp);
accepted.Size = 33;
accepted.Padding = 15;
accepted.Padding = 16;

Assert.IsFalse(rejected.Validate(out _), "Padding of 16 is not less than 33 / 2 == 16.");
Assert.IsTrue(accepted.Validate(out _), "Padding of 15 is less than 33 / 2 == 16.");
Assert.IsFalse(rejected.Validate(out _), "Padding of 17 on 33 pixels leaves no content.");
Assert.IsTrue(accepted.Validate(out _), "Padding of 16 on 33 pixels leaves one pixel of content.");
}

[TestMethod]
[DataRow(1, 0)]
[DataRow(3, 1)]
public void ValidateAcceptsTheSmallestSizesThePipelineHandles(int size, int padding)
{
// Size / 2 rounded these down to a bound of 0 and 1, so --size 1 failed even at the
// default padding of 0, and --size 3 --padding 1 was refused though it leaves a pixel.
using TempDirectory temp = new();
Arguments args = ValidArguments(temp);
args.Size = size;
args.Padding = padding;

bool valid = args.Validate(out Collection<string> errors);

Assert.IsTrue(valid, $"Expected --size {size} --padding {padding} to be valid but got: {string.Join(", ", errors)}");
}

[TestMethod]
public void ValidateRejectsPaddingThatLeavesNoContentOnASmallSize()
{
using TempDirectory temp = new();
Arguments args = ValidArguments(temp);
args.Size = 4;
args.Padding = 2;

bool valid = args.Validate(out Collection<string> errors);

Assert.IsFalse(valid, "Two pixels a side on a four pixel canvas leaves no content.");
Assert.HasCount(1, errors);
Assert.AreEqual("Padding must be less than half the size of the image.", errors[0]);
}

[TestMethod]
Expand Down
7 changes: 5 additions & 2 deletions IconHelper/Arguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal sealed class Arguments
[Option('s', "size", Required = false, HelpText = "The maximum size of the icon. Defaults to 128.")]
public int Size { get; set; } = 128;

[Option('p', "padding", Required = false, HelpText = "The number of pixels per side to pad the output image. Must be < (size / 2), and is clamped further on artwork that downscales to less than size. Will not change the output size. Defaults to 0.")]
[Option('p', "padding", Required = false, HelpText = "The number of pixels per side to pad the output image. Must be less than half the size, leaving at least one pixel of content, and is clamped further on artwork that downscales to less than size. Will not change the output size. Defaults to 0.")]
public int Padding { get; set; } = 0;

internal bool Validate(out Collection<string> errors)
Expand All @@ -50,7 +50,10 @@ internal bool Validate(out Collection<string> errors)
{
errors.Add("Padding must not be negative.");
}
else if (Size > 0 && Padding >= Size / 2)
// Doubled rather than halved: Size / 2 rounds an odd size down, which rejected padding the
// pipeline handles (--size 3 --padding 1) and even the default 0 at --size 1. This is the
// bound EffectivePadding clamps to, (size - 1) / 2, so at least one pixel of content is left.
else if (Size > 0 && Padding * 2 >= Size)
{
errors.Add("Padding must be less than half the size of the image.");
}
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,14 @@ already contains its own output will not reprocess those files.
| `-o` | `--output` | Yes | n/a | Path to the directory where modified files are written |
| `-c` | `--color` | No | `#FFFFFF` | The colour to tint the icon with, as hex or a known name |
| `-s` | `--size` | No | `128` | The maximum size, in pixels, of the output icon |
| `-p` | `--padding` | No | `0` | Pixels of padding per side. Must be less than `size / 2`, and is clamped further on artwork that downscales to less than `size`. Does not change the output dimensions |
| `-p` | `--padding` | No | `0` | Pixels of padding per side. Must be less than half of `size` (`padding * 2 < size`), and is clamped further on artwork that downscales to less than `size`. Does not change the output dimensions |

## Exit Codes

| Code | Meaning |
|------|---------|
| `0` | Every file was processed successfully. Also returned for `--help` and `--version` |
| `1` | The arguments were unusable, for example an `--input` directory that does not exist, an unrecognised `--color`, or `padding >= size / 2` |
| `1` | The arguments were unusable, for example an `--input` directory that does not exist, an unrecognised `--color`, or `padding * 2 >= size` |
| `2` | The batch ran to completion but at least one file could not be processed |

Code `2` means the run finished and the remaining icons were still written. Check the summary line
Expand Down
Loading