mirror of https://github.com/icsharpcode/ILSpy.git
Browse Source
The "Open from NuGet feed" chooser rendered every result with the default NuGet logo, even though the feed search already returns each package's own IconUrl - it was fetched into NuGetPackageInfo and then ignored. The list looked like a wall of identical icons, unlike the NuGet gallery. NuGetPackageInfo is an immutable feed DTO bound straight into the ListBox, so it can't raise PropertyChanged for an icon that arrives later. Rather than bolt mutability and an Avalonia image type onto that DTO, introduce a thin per-row view model (NuGetPackageViewModel) that owns an observable Icon defaulting to the logo and swaps in the real icon once downloaded, plus an INuGetIconLoader service that fetches and decodes off the UI thread. Icons are decoded to 64px (rendered at 32) and cached by URL as the in-flight Task so each URL downloads once and concurrent requests share it; the loader is held for the command's lifetime so the cache survives reopening the dialog. A per-search CancellationTokenSource stops a stale result set's downloads from painting onto rows recycled by the next search. Any failure - non-http URL, network error, decode error - collapses to null so a dead icon URL just keeps the default. Also disable horizontal scrolling on the results list. With it enabled (the default), rows measured at unbounded width, so the star column stopped filling the viewport and selecting a row brought the overflow into view - the list jumped sideways and grew a scrollbar. The row layout is meant to fit the width (the description wraps/trims), so horizontal scrolling was never wanted. Assisted-by: Claude:claude-opus-4-8:Claude Codepull/3813/head
Christoph Wille
2 weeks ago
11 changed files with 461 additions and 45 deletions
@ -0,0 +1,83 @@ |
|||||||
|
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
|
||||||
|
//
|
||||||
|
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
|
||||||
|
// software and associated documentation files (the "Software"), to deal in the Software
|
||||||
|
// without restriction, including without limitation the rights to use, copy, modify, merge,
|
||||||
|
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
|
||||||
|
// to whom the Software is furnished to do so, subject to the following conditions:
|
||||||
|
//
|
||||||
|
// The above copyright notice and this permission notice shall be included in all copies or
|
||||||
|
// substantial portions of the Software.
|
||||||
|
//
|
||||||
|
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
|
||||||
|
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
|
||||||
|
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
|
||||||
|
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
|
||||||
|
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
|
||||||
|
// DEALINGS IN THE SOFTWARE.
|
||||||
|
|
||||||
|
using System; |
||||||
|
using System.Collections.Concurrent; |
||||||
|
using System.Threading; |
||||||
|
using System.Threading.Tasks; |
||||||
|
|
||||||
|
using Avalonia; |
||||||
|
using Avalonia.Media; |
||||||
|
|
||||||
|
using ICSharpCode.ILSpy.NuGetFeeds; |
||||||
|
|
||||||
|
namespace ICSharpCode.ILSpy.Tests.NuGetFeeds; |
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// In-memory <see cref="INuGetIconLoader"/>: records every requested URL and returns a distinct
|
||||||
|
/// placeholder image per URL (or null to model a missing/broken icon), so tests can assert that a
|
||||||
|
/// package's icon was fetched and swapped in without touching the network.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class FakeNuGetIconLoader : INuGetIconLoader |
||||||
|
{ |
||||||
|
public ConcurrentBag<string> RequestedUrls { get; } = new(); |
||||||
|
|
||||||
|
/// <summary>When false, every load resolves to null (models a broken/missing icon).</summary>
|
||||||
|
public bool ReturnImage { get; set; } = true; |
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// When set, loads do not complete until this source is resolved (or the call's token fires),
|
||||||
|
/// letting a test hold icon loads in flight and then cancel them with a new search.
|
||||||
|
/// </summary>
|
||||||
|
public TaskCompletionSource? Gate { get; set; } |
||||||
|
|
||||||
|
public Task<IImage?> LoadIconAsync(string iconUrl, CancellationToken cancellationToken) |
||||||
|
{ |
||||||
|
RequestedUrls.Add(iconUrl); |
||||||
|
if (cancellationToken.IsCancellationRequested) |
||||||
|
return Task.FromResult<IImage?>(null); |
||||||
|
if (Gate != null) |
||||||
|
return AwaitGateAsync(iconUrl, cancellationToken); |
||||||
|
return Task.FromResult(Result(iconUrl)); |
||||||
|
} |
||||||
|
|
||||||
|
async Task<IImage?> AwaitGateAsync(string iconUrl, CancellationToken cancellationToken) |
||||||
|
{ |
||||||
|
// The contract is "cancellation yields a null icon", not an exception; model that so tests
|
||||||
|
// exercise the same path production takes when a search is superseded mid-load.
|
||||||
|
try |
||||||
|
{ |
||||||
|
await Gate!.Task.WaitAsync(cancellationToken); |
||||||
|
} |
||||||
|
catch (OperationCanceledException) |
||||||
|
{ |
||||||
|
return null; |
||||||
|
} |
||||||
|
return Result(iconUrl); |
||||||
|
} |
||||||
|
|
||||||
|
IImage? Result(string iconUrl) => ReturnImage ? new StubImage(iconUrl) : null; |
||||||
|
|
||||||
|
/// <summary>A trivial non-drawing <see cref="IImage"/> that carries the URL it stands in for.</summary>
|
||||||
|
public sealed class StubImage(string url) : IImage |
||||||
|
{ |
||||||
|
public string Url { get; } = url; |
||||||
|
public Size Size => new(64, 64); |
||||||
|
public void Draw(DrawingContext context, Rect sourceRect, Rect destRect) { } |
||||||
|
} |
||||||
|
} |
||||||
@ -0,0 +1,40 @@ |
|||||||
|
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
|
||||||
|
//
|
||||||
|
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
|
||||||
|
// software and associated documentation files (the "Software"), to deal in the Software
|
||||||
|
// without restriction, including without limitation the rights to use, copy, modify, merge,
|
||||||
|
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
|
||||||
|
// to whom the Software is furnished to do so, subject to the following conditions:
|
||||||
|
//
|
||||||
|
// The above copyright notice and this permission notice shall be included in all copies or
|
||||||
|
// substantial portions of the Software.
|
||||||
|
//
|
||||||
|
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
|
||||||
|
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
|
||||||
|
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
|
||||||
|
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
|
||||||
|
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
|
||||||
|
// DEALINGS IN THE SOFTWARE.
|
||||||
|
|
||||||
|
using System.Threading; |
||||||
|
using System.Threading.Tasks; |
||||||
|
|
||||||
|
using Avalonia.Media; |
||||||
|
|
||||||
|
namespace ICSharpCode.ILSpy.NuGetFeeds |
||||||
|
{ |
||||||
|
/// <summary>
|
||||||
|
/// Fetches a package's custom icon for the "Open from NuGet feed" dialog. The production
|
||||||
|
/// implementation (<see cref="NuGetIconLoader"/>) downloads and decodes the image; tests
|
||||||
|
/// substitute a fake so no network is involved.
|
||||||
|
/// </summary>
|
||||||
|
public interface INuGetIconLoader |
||||||
|
{ |
||||||
|
/// <summary>
|
||||||
|
/// Downloads and decodes the icon at <paramref name="iconUrl"/>. Returns
|
||||||
|
/// <see langword="null"/> for anything that isn't a usable icon - a non-http(s) URL, a
|
||||||
|
/// network or decode failure, or cancellation - so callers simply keep the default icon.
|
||||||
|
/// </summary>
|
||||||
|
Task<IImage?> LoadIconAsync(string iconUrl, CancellationToken cancellationToken); |
||||||
|
} |
||||||
|
} |
||||||
@ -0,0 +1,98 @@ |
|||||||
|
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
|
||||||
|
//
|
||||||
|
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
|
||||||
|
// software and associated documentation files (the "Software"), to deal in the Software
|
||||||
|
// without restriction, including without limitation the rights to use, copy, modify, merge,
|
||||||
|
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
|
||||||
|
// to whom the Software is furnished to do so, subject to the following conditions:
|
||||||
|
//
|
||||||
|
// The above copyright notice and this permission notice shall be included in all copies or
|
||||||
|
// substantial portions of the Software.
|
||||||
|
//
|
||||||
|
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
|
||||||
|
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
|
||||||
|
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
|
||||||
|
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
|
||||||
|
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
|
||||||
|
// DEALINGS IN THE SOFTWARE.
|
||||||
|
|
||||||
|
using System; |
||||||
|
using System.Collections.Concurrent; |
||||||
|
using System.IO; |
||||||
|
using System.Net.Http; |
||||||
|
using System.Threading; |
||||||
|
using System.Threading.Tasks; |
||||||
|
|
||||||
|
using Avalonia.Media; |
||||||
|
using Avalonia.Media.Imaging; |
||||||
|
|
||||||
|
namespace ICSharpCode.ILSpy.NuGetFeeds |
||||||
|
{ |
||||||
|
/// <summary>
|
||||||
|
/// Downloads package icons over HTTP and decodes them into Avalonia bitmaps. Results are
|
||||||
|
/// cached by URL for the loader's lifetime, so re-searching or re-displaying the same package
|
||||||
|
/// reuses the already-fetched icon and each URL is downloaded at most once. Icons are decoded
|
||||||
|
/// to a small fixed width since the dialog renders them at 32px.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class NuGetIconLoader : INuGetIconLoader |
||||||
|
{ |
||||||
|
// Rendered at 32px in the list and the detail header; 64px keeps it crisp on HiDPI
|
||||||
|
// without holding the often-large published icon (128px+) in memory per result.
|
||||||
|
const int DecodeWidth = 64; |
||||||
|
|
||||||
|
// Cap how much an icon response may buffer into memory; a broken or hostile feed could
|
||||||
|
// otherwise stream an unbounded payload. Package icons are tiny, so a few MB is generous
|
||||||
|
// and GetByteArrayAsync throws (caught below) once a response exceeds it.
|
||||||
|
const int MaxResponseBytes = 4 * 1024 * 1024; |
||||||
|
|
||||||
|
static readonly HttpClient httpClient = new() { |
||||||
|
Timeout = TimeSpan.FromSeconds(15), |
||||||
|
MaxResponseContentBufferSize = MaxResponseBytes |
||||||
|
}; |
||||||
|
|
||||||
|
// The cached value is the in-flight (or completed) decode task, so concurrent requests for
|
||||||
|
// one URL share a single download instead of racing. A failed/missing icon caches as a
|
||||||
|
// completed null - we don't retry a dead URL within a session.
|
||||||
|
readonly ConcurrentDictionary<string, Task<IImage?>> cache = |
||||||
|
new(StringComparer.OrdinalIgnoreCase); |
||||||
|
|
||||||
|
public async Task<IImage?> LoadIconAsync(string iconUrl, CancellationToken cancellationToken) |
||||||
|
{ |
||||||
|
if (!Uri.TryCreate(iconUrl, UriKind.Absolute, out var uri) |
||||||
|
|| (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps)) |
||||||
|
{ |
||||||
|
return null; |
||||||
|
} |
||||||
|
// One shared download per URL feeds every caller, so the cache dedups regardless of who
|
||||||
|
// asked. Each caller awaits it through its own token, though, so a superseded search
|
||||||
|
// stops waiting at once and gets null (per the interface contract) instead of hanging
|
||||||
|
// on the in-flight download until HttpClient.Timeout.
|
||||||
|
var download = cache.GetOrAdd(iconUrl, |
||||||
|
static (url, client) => DownloadAndDecodeAsync(client, url), httpClient); |
||||||
|
try |
||||||
|
{ |
||||||
|
return await download.WaitAsync(cancellationToken).ConfigureAwait(false); |
||||||
|
} |
||||||
|
catch (OperationCanceledException) |
||||||
|
{ |
||||||
|
return null; |
||||||
|
} |
||||||
|
} |
||||||
|
|
||||||
|
static async Task<IImage?> DownloadAndDecodeAsync(HttpClient client, string url) |
||||||
|
{ |
||||||
|
try |
||||||
|
{ |
||||||
|
byte[] bytes = await client.GetByteArrayAsync(url).ConfigureAwait(false); |
||||||
|
using var stream = new MemoryStream(bytes); |
||||||
|
return Bitmap.DecodeToWidth(stream, DecodeWidth); |
||||||
|
} |
||||||
|
catch (Exception) |
||||||
|
{ |
||||||
|
// A broken or unreachable icon URL is not an error worth surfacing: the dialog
|
||||||
|
// just keeps showing the default NuGet icon for that package.
|
||||||
|
return null; |
||||||
|
} |
||||||
|
} |
||||||
|
} |
||||||
|
} |
||||||
@ -0,0 +1,73 @@ |
|||||||
|
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
|
||||||
|
//
|
||||||
|
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
|
||||||
|
// software and associated documentation files (the "Software"), to deal in the Software
|
||||||
|
// without restriction, including without limitation the rights to use, copy, modify, merge,
|
||||||
|
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
|
||||||
|
// to whom the Software is furnished to do so, subject to the following conditions:
|
||||||
|
//
|
||||||
|
// The above copyright notice and this permission notice shall be included in all copies or
|
||||||
|
// substantial portions of the Software.
|
||||||
|
//
|
||||||
|
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
|
||||||
|
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
|
||||||
|
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
|
||||||
|
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
|
||||||
|
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
|
||||||
|
// DEALINGS IN THE SOFTWARE.
|
||||||
|
|
||||||
|
using System; |
||||||
|
using System.Threading; |
||||||
|
using System.Threading.Tasks; |
||||||
|
|
||||||
|
using Avalonia.Media; |
||||||
|
|
||||||
|
using CommunityToolkit.Mvvm.ComponentModel; |
||||||
|
|
||||||
|
using ICSharpCode.ILSpy.NuGetFeeds; |
||||||
|
|
||||||
|
namespace ICSharpCode.ILSpy.ViewModels |
||||||
|
{ |
||||||
|
/// <summary>
|
||||||
|
/// One row in the "Open from NuGet feed" results list: the immutable feed
|
||||||
|
/// <see cref="NuGetPackageInfo"/> plus the observable <see cref="Icon"/>. The icon starts as
|
||||||
|
/// the default NuGet logo and is swapped for the package's own icon once it has been fetched,
|
||||||
|
/// so the list can render immediately and fill in custom icons as they arrive.
|
||||||
|
/// </summary>
|
||||||
|
public sealed partial class NuGetPackageViewModel : ObservableObject |
||||||
|
{ |
||||||
|
public NuGetPackageInfo Info { get; } |
||||||
|
|
||||||
|
[ObservableProperty] |
||||||
|
IImage icon = Images.NuGet; |
||||||
|
|
||||||
|
public NuGetPackageViewModel(NuGetPackageInfo info) |
||||||
|
{ |
||||||
|
Info = info; |
||||||
|
} |
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Fetches the package's custom icon in the background and, on success, replaces the
|
||||||
|
/// default icon. A package without an icon URL keeps the default and never hits the loader.
|
||||||
|
/// </summary>
|
||||||
|
public async Task LoadIconAsync(INuGetIconLoader loader, CancellationToken cancellationToken) |
||||||
|
{ |
||||||
|
if (string.IsNullOrEmpty(Info.IconUrl)) |
||||||
|
return; |
||||||
|
try |
||||||
|
{ |
||||||
|
var image = await loader.LoadIconAsync(Info.IconUrl, cancellationToken); |
||||||
|
// The await resumes on the UI thread (the dialog view model drives this from
|
||||||
|
// there), so assigning the observable Icon raises PropertyChanged safely.
|
||||||
|
if (image != null && !cancellationToken.IsCancellationRequested) |
||||||
|
Icon = image; |
||||||
|
} |
||||||
|
catch (Exception) |
||||||
|
{ |
||||||
|
// This runs fire-and-forget, so nothing observes the returned task: any failure
|
||||||
|
// (cancellation from a superseded search, or a misbehaving loader) must be swallowed
|
||||||
|
// here rather than surface as an unobserved task exception. Keep the default icon.
|
||||||
|
} |
||||||
|
} |
||||||
|
} |
||||||
|
} |
||||||
Loading…
Reference in new issue