Skip to content

Feature/rlsprep #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
trackd merged 2 commits into from
Feb 16, 2026
Merged

Feature/rlsprep #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aab7067
feat: ✨ Add timeout parameter to `ConvertTo-Sixel` and `ConvertTo-Six…
trackd Feb 16, 2026
181c9cc
better cleanup
trackd Feb 16, 2026
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
4 changes: 2 additions & 2 deletions build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ if (-not (Get-Module -ListAvailable -Name InvokeBuild)) {
}
Import-Module InvokeBuild -ErrorAction Stop

if ($task) {
$buildparams.Task = $task
if ($Task) {
$buildparams.Task = $Task
}

if (-not $env:CI) {
Expand Down
18 changes: 17 additions & 1 deletion docs/en-US/ConvertTo-Sixel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ ConvertTo-Sixel [-Path] <String> [-MaxColors <int>] [-Width <int>] [-Height <int
### Url

```powershell
ConvertTo-Sixel -Url <Uri> [-MaxColors <int>] [-Width <int>] [-Height <int>] [-Force] [<CommonParameters>]
ConvertTo-Sixel -Url <Uri> [-MaxColors <int>] [-Width <int>] [-Height <int>] [-Force] [-Timeout <TimeSpan>] [<CommonParameters>]
```

### Stream
Expand Down Expand Up @@ -214,6 +214,22 @@ Accept pipeline input: False
Accept wildcard characters: False
```

### -Timeout

Timeout for webrequest
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling: “webrequest” should be “web request”.

Suggested change
Timeout for webrequest
Timeout for web request

Copilot uses AI. Check for mistakes.

```yaml
Type: TimeSpan
Parameter Sets: Url
Aliases:

Required: False
Position: Named
Default value: 15
Accept pipeline input: False
Accept wildcard characters: False
```

## INPUTS

### System.String
Expand Down
18 changes: 17 additions & 1 deletion docs/en-US/ConvertTo-SixelGif.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ ConvertTo-SixelGif [-Path] <string> [-MaxColors <int>] [-Width <int>] [-Force] [
### Url

```powershell
ConvertTo-SixelGif -Url <uri> [-MaxColors <int>] [-Width <int>] [-Force] [-LoopCount <int>] [<CommonParameters>]
ConvertTo-SixelGif -Url <uri> [-MaxColors <int>] [-Width <int>] [-Force] [-LoopCount <int>] [-Timeout <TimeSpan>] [<CommonParameters>]
```

### Stream
Expand Down Expand Up @@ -174,6 +174,22 @@ Accept pipeline input: False
Accept wildcard characters: False
```

### -Timeout

Timeout for webrequest
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling: “webrequest” should be “web request”.

Suggested change
Timeout for webrequest
Timeout for web request

Copilot uses AI. Check for mistakes.

```yaml
Type: TimeSpan
Parameter Sets: Url
Aliases:

Required: False
Position: Named
Default value: 15
Accept pipeline input: False
Accept wildcard characters: False
```

## INPUTS

### System.String
Expand Down
2 changes: 1 addition & 1 deletion module/Sixel.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
ProjectUri = 'https://github.com/trackd/Sixel'
# Prerelease = 'prerelease01'
ReleaseNotes = @'
0.7.0 - update libraries, bugfixes.
0.7.0 - new Braille protocol support, improved terminal detection, and library updates.
0.6.1 - bugfix resizing image sometimes cuts off the left outer edge.
0.6.0 - update libraries, tweak sizing algorithm.
0.5.0 - Refactor, cleanup, bugfixes with terminal detection and stream.
Expand Down
11 changes: 9 additions & 2 deletions src/Sixel/Cmdlet/ConvertToSixel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Sixel.Cmdlet;
[Alias("cts")]
[OutputType(typeof(string))]
public sealed class ConvertSixelCmdlet : PSCmdlet {
private static readonly HttpClient _httpClient = new() { Timeout = TimeSpan.FromSeconds(30) };
private static readonly HttpClient _httpClient = new();
[Parameter(
HelpMessage = "InputObject from Pipeline, can be filepath or base64 encoded image.",
Mandatory = true,
Expand Down Expand Up @@ -66,7 +66,6 @@ public sealed class ConvertSixelCmdlet : PSCmdlet {
[ValidateTerminalWidth()]
public int Width { get; set; }


[Parameter(
HelpMessage = "Height of the image in character cells, the width will be scaled to maintain aspect ratio."
)]
Expand All @@ -83,6 +82,12 @@ public sealed class ConvertSixelCmdlet : PSCmdlet {
)]
public ImageProtocol Protocol { get; set; } = ImageProtocol.Auto;

[Parameter(
HelpMessage = "Timeout for web request",
ParameterSetName = "Url"
)]
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timeout has no validation. Passing TimeSpan.Zero or a negative value will throw when assigned to HttpClient.Timeout, turning a parameter validation issue into a runtime error. Consider adding PowerShell validation (e.g., ValidateScript to enforce > TimeSpan.Zero or allow Timeout.InfiniteTimeSpan) before using it.

Suggested change
)]
)]
[ValidateScript({
// Validate that the timeout is strictly greater than zero or is InfiniteTimeSpan.
// This prevents runtime ArgumentOutOfRangeException when assigning to HttpClient.Timeout.
if ($_ -gt [TimeSpan]::Zero -or $_ -eq [System.Threading.Timeout]::InfiniteTimeSpan) {
return $true;
}
throw [System.ArgumentOutOfRangeException]::new(
'Timeout',
$_,
'Timeout must be greater than [TimeSpan]::Zero or equal to [System.Threading.Timeout]::InfiniteTimeSpan.'
);
})]

Copilot uses AI. Check for mistakes.
public TimeSpan Timeout { get; set; } = TimeSpan.FromSeconds(15);

Comment on lines +85 to +90
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New -Timeout behavior is only implemented for the Url parameter set, but there are no tests covering Url downloads (and therefore no coverage that -Timeout is honored). Add a Pester test that exercises ConvertTo-Sixel -Url ... -Timeout ... (ideally against a local test server or a controlled slow endpoint) so regressions are caught.

Copilot generated this review using guidance from repository custom instructions.
protected override void ProcessRecord() {
Stream? imageStream = null;
try {
Expand All @@ -106,6 +111,7 @@ protected override void ProcessRecord() {
}
break;
case "Url": {
_httpClient.Timeout = Timeout;
HttpResponseMessage response = _httpClient.GetAsync(Url).GetAwaiter().GetResult();
_ = response.EnsureSuccessStatusCode();
Comment on lines 113 to 116
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Timeout parameter is applied by mutating a shared static HttpClient instance (_httpClient.Timeout = Timeout). In PowerShell it’s common to run cmdlets concurrently (multiple runspaces), so this can cause races where one invocation changes the timeout for another. Prefer a per-request timeout via CancellationTokenSource.CancelAfter(...), or avoid sharing/mutating a static client (e.g., instance client per cmdlet invocation).

Copilot uses AI. Check for mistakes.
imageStream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult();
Comment on lines 115 to 117
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HttpResponseMessage response is never disposed in the Url parameter set. Because the response owns the underlying connection, this can lead to socket/connection leaks under repeated use. Restructure so the response is disposed after the content stream is fully consumed (e.g., buffer into a MemoryStream and dispose the response, or track response and dispose it in finally along with the stream).

Copilot uses AI. Check for mistakes.
Expand All @@ -119,6 +125,7 @@ protected override void ProcessRecord() {
break;
}
default:
// just to stop analyzer from complaining..
break;
}
if (imageStream is null) {
Expand Down
16 changes: 13 additions & 3 deletions src/Sixel/Cmdlet/ConvertToSixelGif.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Management.Automation;
using System.Management.Automation;
using System.Net.Http;
using Sixel.Protocols;
using Sixel.Terminal;
Expand All @@ -11,7 +11,8 @@ namespace Sixel.Cmdlet;
[Alias("gif")]
[OutputType(typeof(SixelGif))]
public sealed class ConvertSixelGifCmdlet : PSCmdlet {
private static readonly HttpClient _httpClient = new() { Timeout = TimeSpan.FromSeconds(30) };
private static readonly HttpClient _httpClient = new();

[Parameter(
HelpMessage = "InputObject from Pipeline, can be filepath or base64 encoded image.",
Mandatory = true,
Expand Down Expand Up @@ -75,7 +76,14 @@ public sealed class ConvertSixelGifCmdlet : PSCmdlet {
[Parameter(
HelpMessage = "The number of times to loop the gif. Use 0 for infinite loop."
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LoopCount help text says “Use 0 for infinite loop”, and the new validation explicitly allows 0, but PlaySixelGif currently loops for (int i = 0; i < gif.LoopCount; i++), so LoopCount = 0 will play 0 times (not infinite). Either implement the special-case semantics for 0, or update the parameter help/validation to match the actual behavior.

Suggested change
HelpMessage = "The number of times to loop the gif. Use 0 for infinite loop."
HelpMessage = "The number of times to loop the gif."

Copilot uses AI. Check for mistakes.
)]
[ValidateRange(0, 256)]
public int LoopCount { get; set; } = 3;

[Parameter(
HelpMessage = "Timeout for web request",
ParameterSetName = "Url"
)]
public TimeSpan Timeout { get; set; } = TimeSpan.FromSeconds(15);
Comment on lines +82 to +86
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timeout has no validation. Passing TimeSpan.Zero or a negative value will throw when assigned to HttpClient.Timeout, resulting in a runtime error instead of a parameter validation error. Consider adding PowerShell validation (e.g., ValidateScript to enforce > TimeSpan.Zero or allow Timeout.InfiniteTimeSpan).

Copilot uses AI. Check for mistakes.
protected override void ProcessRecord() {
Comment on lines +82 to 87
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New -Timeout behavior is only implemented for the Url parameter set, but there are no tests covering Url downloads (and therefore no coverage that -Timeout is honored). Add a Pester test that exercises ConvertTo-SixelGif -Url ... -Timeout ... (ideally against a local test server or a controlled slow endpoint).

Copilot generated this review using guidance from repository custom instructions.
Stream? imageStream = null;
try {
Expand All @@ -100,8 +108,9 @@ protected override void ProcessRecord() {
}
break;
case "Url": {
_httpClient.Timeout = Timeout;
HttpResponseMessage response = _httpClient.GetAsync(Url).GetAwaiter().GetResult();
response.EnsureSuccessStatusCode();
_ = response.EnsureSuccessStatusCode();
Comment on lines 110 to +113
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Timeout parameter is applied by mutating a shared static HttpClient instance (_httpClient.Timeout = Timeout). In concurrent PowerShell usage this can cause races where one invocation changes the timeout for another. Prefer a per-request timeout via CancellationTokenSource.CancelAfter(...), or avoid sharing/mutating a static client.

Copilot uses AI. Check for mistakes.
imageStream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult();
Comment on lines 112 to 114
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HttpResponseMessage response is never disposed in the Url parameter set. Since the response owns the underlying connection, repeated use can leak sockets/connections. Restructure so the response is disposed after the content stream is consumed (e.g., buffer into a MemoryStream and dispose the response, or dispose response in finally).

Copilot uses AI. Check for mistakes.
break;
}
Expand All @@ -113,6 +122,7 @@ protected override void ProcessRecord() {
break;
}
default:
// just to stop analyzer from complaining..
break;
}
if (imageStream is null) {
Expand Down
17 changes: 14 additions & 3 deletions src/Sixel/Helpers/ResizerDev.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,21 @@

namespace Sixel.Terminal;

/// <summary>
/// testing math..
/// Experimental image resizing helpers used for validating and tuning terminal rendering math.
/// </summary>
public static class ResizerDev {
/// <remarks>
/// <para>
/// <see cref="ResizerDev"/> provides alternative resizing algorithms and size calculations
/// intended primarily for development, testing, and comparison against the main resizing
/// helpers in this library (for example, non-<c>Dev</c> resizer/size helper classes).
/// </para>
/// <para>
/// This API is considered <b>experimental</b>: its behavior and surface area may change
/// between releases without notice. Consumers should prefer the primary, documented
/// resizing helpers for production use, and treat this type as an advanced or diagnostic
/// utility.
/// </para>
internal static class ResizerDev {
public static Image<Rgba32> ResizeForSixel(
Comment on lines +23 to 24
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this type from public to internal is a breaking API change for any external consumers referencing the Sixel assembly. If this is intentional (as the remarks suggest), it should be reflected in the library’s versioning/release notes to set expectations for downstream users.

Copilot uses AI. Check for mistakes.
Image<Rgba32> image,
ImageSize imageSize,
Expand Down
10 changes: 7 additions & 3 deletions src/Sixel/Helpers/SizeHelperDev.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@

namespace Sixel.Terminal;

/// <summary>
/// testing math..
/// Provides experimental image sizing helpers for terminal rendering.
/// </summary>
public static class SizeHelperDev {
/// <remarks>
/// This class contains development and testing implementations of image sizing logic
/// used when rendering images in terminal character cells. It is intended for
/// experimentation and validation of sizing math and may change between releases.
/// </remarks>
internal static class SizeHelperDev {
public static ImageSize GetSixelTargetSize(Image<Rgba32> image, int maxCellWidth, int maxCellHeight)
=> GetRequestedOrDefaultCellSize(image, maxCellWidth, maxCellHeight);

Expand Down
13 changes: 12 additions & 1 deletion src/Sixel/Protocols/Blocks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,16 @@ private static void AppendBlock(this StringBuilder Builder, byte tr, byte tg, by
Append(Constants.Reset);
}

private static bool IsTransparent(Rgba32 pixel) => pixel.A == 0;
// private static bool IsTransparent(Rgba32 pixel) => pixel.A == 0;
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a commented-out IsTransparent implementation left in the file. Keeping dead code as comments makes future maintenance harder; it’s better to remove it (version control already preserves history).

Suggested change
// private static bool IsTransparent(Rgba32 pixel) => pixel.A == 0;

Copilot uses AI. Check for mistakes.
private static bool IsTransparent(Rgba32 pixel) {
if (pixel.A <= 8) {
return true;
}

float luminance = ((0.299f * pixel.R) + (0.587f * pixel.G) + (0.114f * pixel.B)) / 255f;
return (pixel.A < 32 && luminance < 0.15f) ||
(pixel.A < 64 && pixel.R < 12 && pixel.G < 12 && pixel.B < 12) ||
(pixel.A < 128 && luminance < 0.05f) ||
(pixel.A < 240 && luminance < 0.01f);
}
}
10 changes: 5 additions & 5 deletions src/Sixel/Protocols/Braille.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ private static void AppendCodepoint(this StringBuilder Builder, int codepoint, b
Append((char)codepoint).
Append(Constants.Reset);
}
private static bool IsTransparent(Rgba32 pixel) => pixel.A == 0;
private static bool IsTransparentAdv(Rgba32 pixel) {
if (pixel.A == 0) {

// private static bool IsTransparent(Rgba32 pixel) => pixel.A == 0;
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a commented-out IsTransparent implementation left in the file. Keeping dead code as comments makes future maintenance harder; it’s better to remove it (version control already preserves history) or convert it into a proper alternative implementation behind a feature flag if it’s still needed.

Suggested change
// private static bool IsTransparent(Rgba32 pixel) => pixel.A == 0;

Copilot uses AI. Check for mistakes.
private static bool IsTransparent(Rgba32 pixel) {
if (pixel.A <= 8) {
return true;
}

float luminance = ((0.299f * pixel.R) + (0.587f * pixel.G) + (0.114f * pixel.B)) / 255f;
return pixel.A < 8 ||
(pixel.A < 32 && luminance < 0.15f) ||
return (pixel.A < 32 && luminance < 0.15f) ||
(pixel.A < 64 && pixel.R < 12 && pixel.G < 12 && pixel.B < 12) ||
(pixel.A < 128 && luminance < 0.05f) ||
(pixel.A < 240 && luminance < 0.01f);
Expand Down
2 changes: 1 addition & 1 deletion src/Sixel/Sixel.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
</PropertyGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
<PackageReference Include="SixLabors.ImageSharp" Version="3.1.12" />
<PackageReference Include="System.Management.Automation" Version="7.4" PrivateAssets="all" />
<PackageReference Include="System.Management.Automation" Version="7.4.*" PrivateAssets="all" />
<PackageReference Include="Roslynator.Analyzers" Version="4.*">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
Expand Down
Loading
Loading