Browse Source

Reject the name of an existing list in the assembly list prompt

The WPF host kept the name prompt open and said the name was taken; the port
kept the resource string but dropped the message, and the New / Clone / Rename
/ Add-preconfigured handlers only return early on a collision. Nothing tells
the user why the list they just named did not appear, which reads as the dialog
having accepted the name.

The check belongs in the prompt, where the name is entered: OK stays disabled
while the name collides, so no flow can be handed one. Rename passes the name
of the list being renamed as allowed, because that is a no-op rather than a
collision with itself.

Assisted-by: Claude:claude-opus-5:Claude Code
pull/4129/head
Siegfried Pammer 7 days ago
parent
commit
5d3bb2e031
  1. 104
      ILSpy.Tests/AssemblyList/ManageAssemblyListsNameValidationTests.cs
  2. 2
      ILSpy/Views/CreateListDialog.axaml
  3. 26
      ILSpy/Views/CreateListDialog.axaml.cs
  4. 19
      ILSpy/Views/ManageAssemblyListsDialog.axaml.cs

104
ILSpy.Tests/AssemblyList/ManageAssemblyListsNameValidationTests.cs

@ -0,0 +1,104 @@ @@ -0,0 +1,104 @@
// Copyright (c) 2026 Siegfried Pammer
//
// 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 Avalonia.Controls;
using Avalonia.Headless.NUnit;
using Avalonia.Threading;
using AwesomeAssertions;
using ICSharpCode.ILSpy;
using ICSharpCode.ILSpy.AppEnv;
using ICSharpCode.ILSpy.Views;
using NUnit.Framework;
namespace ICSharpCode.ILSpy.Tests;
/// <summary>
/// Manage Assembly Lists: a new list may not take the name of an existing one. The prompt has
/// to say so and keep OK disabled - the operations behind it silently do nothing for a name
/// that is taken, which reads as the dialog having accepted the name.
/// </summary>
[TestFixture]
public class ManageAssemblyListsNameValidationTests
{
static (ManageAssemblyListsDialog Dialog, string TakenName) DialogWithOneList()
{
var settingsService = AppComposition.Current.GetExport<SettingsService>();
var manager = settingsService.AssemblyListManager;
var taken = "List " + Guid.NewGuid().ToString("N");
manager.AddListIfNotExists(manager.CreateList(taken));
return (new ManageAssemblyListsDialog(settingsService), taken);
}
static (TextBox Name, Button Ok, TextBlock Message) Controls(CreateListDialog prompt)
=> (prompt.FindControl<TextBox>("ListNameBox")!,
prompt.FindControl<Button>("OkButton")!,
prompt.FindControl<TextBlock>("NameTakenText")!);
[AvaloniaTest]
public void Prompt_Rejects_The_Name_Of_An_Existing_List()
{
var (dialog, taken) = DialogWithOneList();
var prompt = dialog.CreatePrompt("New Assembly List");
prompt.Show();
try
{
var (name, ok, message) = Controls(prompt);
name.Text = taken;
Dispatcher.UIThread.RunJobs();
ok.IsEnabled.Should().BeFalse("the name is already in use");
message.IsVisible.Should().BeTrue("the user has to be told why OK is disabled");
name.Text = taken + " (2)";
Dispatcher.UIThread.RunJobs();
ok.IsEnabled.Should().BeTrue("the name is free");
message.IsVisible.Should().BeFalse();
}
finally
{
prompt.Close();
dialog.Close();
}
}
[AvaloniaTest]
public void Rename_Accepts_The_Name_The_List_Already_Has()
{
var (dialog, taken) = DialogWithOneList();
// Renaming a list to its own name is a no-op, not a collision with itself.
var prompt = dialog.CreatePrompt("Rename Assembly List", taken, allowedName: taken);
prompt.Show();
try
{
var (_, ok, message) = Controls(prompt);
Dispatcher.UIThread.RunJobs();
ok.IsEnabled.Should().BeTrue();
message.IsVisible.Should().BeFalse();
}
finally
{
prompt.Close();
dialog.Close();
}
}
}

2
ILSpy/Views/CreateListDialog.axaml

@ -8,6 +8,8 @@ @@ -8,6 +8,8 @@
<Grid Margin="12,8" RowDefinitions="Auto,Auto,*,Auto">
<TextBlock Grid.Row="0" Text="Enter the name of the list:" Margin="0,0,0,6" />
<TextBox Grid.Row="1" Name="ListNameBox" Margin="0,4" />
<TextBlock Grid.Row="2" Name="NameTakenText" VerticalAlignment="Top"
Foreground="Red" TextWrapping="Wrap" IsVisible="False" />
<StackPanel Grid.Row="3" Orientation="Horizontal" HorizontalAlignment="Right" Spacing="6">
<Button Name="OkButton" Content="OK" IsDefault="True" IsEnabled="False" MinWidth="72" />
<Button Name="CancelButton" Content="Cancel" IsCancel="True" MinWidth="72" />

26
ILSpy/Views/CreateListDialog.axaml.cs

@ -16,6 +16,8 @@ @@ -16,6 +16,8 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System;
using Avalonia.Controls;
using Avalonia.Markup.Xaml;
@ -32,25 +34,45 @@ namespace ICSharpCode.ILSpy.Views @@ -32,25 +34,45 @@ namespace ICSharpCode.ILSpy.Views
{
TextBox listNameBox = null!;
Button okButton = null!;
TextBlock nameTakenText = null!;
Func<string, bool> isNameTaken = static _ => false;
public CreateListDialog()
{
InitializeComponent();
listNameBox = this.FindControl<TextBox>("ListNameBox")!;
okButton = this.FindControl<Button>("OkButton")!;
listNameBox.TextChanged += (_, _) => okButton.IsEnabled = !string.IsNullOrWhiteSpace(listNameBox.Text);
nameTakenText = this.FindControl<TextBlock>("NameTakenText")!;
nameTakenText.Text = Properties.Resources.ListExistsAlready;
listNameBox.TextChanged += (_, _) => Validate();
okButton.Click += (_, _) => Close(listNameBox.Text);
((Button)this.FindControl<Button>("CancelButton")!).Click += (_, _) => Close(null);
}
public CreateListDialog(string title, string? initialText = null) : this()
/// <param name="isNameTaken">
/// Decides whether the entered name is already in use. Every caller writes into the same
/// set of assembly-list names, so a name that is taken cannot be accepted: the operation
/// behind the prompt would do nothing at all and the dialog would look like it had
/// worked. OK stays disabled for as long as the name collides.
/// </param>
public CreateListDialog(string title, string? initialText = null, Func<string, bool>? isNameTaken = null) : this()
{
Title = title;
this.isNameTaken = isNameTaken ?? this.isNameTaken;
if (!string.IsNullOrEmpty(initialText))
{
listNameBox.Text = initialText;
listNameBox.SelectAll();
}
Validate();
}
void Validate()
{
var name = listNameBox.Text;
bool taken = !string.IsNullOrWhiteSpace(name) && isNameTaken(name);
nameTakenText.IsVisible = taken;
okButton.IsEnabled = !string.IsNullOrWhiteSpace(name) && !taken;
}
void InitializeComponent() => AvaloniaXamlLoader.Load(this);

19
ILSpy/Views/ManageAssemblyListsDialog.axaml.cs

@ -106,11 +106,18 @@ namespace ICSharpCode.ILSpy.Views @@ -106,11 +106,18 @@ namespace ICSharpCode.ILSpy.Views
/// <summary>The lists control, exposed for tests to drive the selection.</summary>
internal ListBox ListsControl => listsBox;
async Task<string?> PromptAsync(string title, string? initialText = null)
{
var dlg = new CreateListDialog(title, initialText);
return await dlg.ShowDialog<string?>(this);
}
async Task<string?> PromptAsync(string title, string? initialText = null, string? allowedName = null)
=> await CreatePrompt(title, initialText, allowedName).ShowDialog<string?>(this);
/// <summary>
/// Builds the name prompt for one of the CRUD flows. <paramref name="allowedName"/> is the
/// name the flow may keep - the name of the list being renamed - which is not a collision
/// with itself. Separated from <see cref="PromptAsync"/> so it can be driven by tests
/// without a modal window.
/// </summary>
internal CreateListDialog CreatePrompt(string title, string? initialText = null, string? allowedName = null)
=> new CreateListDialog(title, initialText,
name => name != allowedName && manager.AssemblyLists.Contains(name));
async Task NewListAsync()
{
@ -135,7 +142,7 @@ namespace ICSharpCode.ILSpy.Views @@ -135,7 +142,7 @@ namespace ICSharpCode.ILSpy.Views
{
if (SelectedListName is not { } selected)
return;
var name = await PromptAsync("Rename Assembly List", selected);
var name = await PromptAsync("Rename Assembly List", selected, allowedName: selected);
if (string.IsNullOrWhiteSpace(name) || name == selected || manager.AssemblyLists.Contains(name))
return;
manager.RenameList(selected, name);

Loading…
Cancel
Save