Skip to content

Commit 5d3bb2e

Browse files
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
1 parent addf4ce commit 5d3bb2e

4 files changed

Lines changed: 143 additions & 8 deletions

File tree

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// Copyright (c) 2026 Siegfried Pammer
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
4+
// software and associated documentation files (the "Software"), to deal in the Software
5+
// without restriction, including without limitation the rights to use, copy, modify, merge,
6+
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
7+
// to whom the Software is furnished to do so, subject to the following conditions:
8+
//
9+
// The above copyright notice and this permission notice shall be included in all copies or
10+
// substantial portions of the Software.
11+
//
12+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
13+
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
14+
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
15+
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
16+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
17+
// DEALINGS IN THE SOFTWARE.
18+
19+
using System;
20+
21+
using Avalonia.Controls;
22+
using Avalonia.Headless.NUnit;
23+
using Avalonia.Threading;
24+
25+
using AwesomeAssertions;
26+
27+
using ICSharpCode.ILSpy;
28+
using ICSharpCode.ILSpy.AppEnv;
29+
using ICSharpCode.ILSpy.Views;
30+
31+
using NUnit.Framework;
32+
33+
namespace ICSharpCode.ILSpy.Tests;
34+
35+
/// <summary>
36+
/// Manage Assembly Lists: a new list may not take the name of an existing one. The prompt has
37+
/// to say so and keep OK disabled - the operations behind it silently do nothing for a name
38+
/// that is taken, which reads as the dialog having accepted the name.
39+
/// </summary>
40+
[TestFixture]
41+
public class ManageAssemblyListsNameValidationTests
42+
{
43+
static (ManageAssemblyListsDialog Dialog, string TakenName) DialogWithOneList()
44+
{
45+
var settingsService = AppComposition.Current.GetExport<SettingsService>();
46+
var manager = settingsService.AssemblyListManager;
47+
var taken = "List " + Guid.NewGuid().ToString("N");
48+
manager.AddListIfNotExists(manager.CreateList(taken));
49+
return (new ManageAssemblyListsDialog(settingsService), taken);
50+
}
51+
52+
static (TextBox Name, Button Ok, TextBlock Message) Controls(CreateListDialog prompt)
53+
=> (prompt.FindControl<TextBox>("ListNameBox")!,
54+
prompt.FindControl<Button>("OkButton")!,
55+
prompt.FindControl<TextBlock>("NameTakenText")!);
56+
57+
[AvaloniaTest]
58+
public void Prompt_Rejects_The_Name_Of_An_Existing_List()
59+
{
60+
var (dialog, taken) = DialogWithOneList();
61+
var prompt = dialog.CreatePrompt("New Assembly List");
62+
prompt.Show();
63+
try
64+
{
65+
var (name, ok, message) = Controls(prompt);
66+
67+
name.Text = taken;
68+
Dispatcher.UIThread.RunJobs();
69+
ok.IsEnabled.Should().BeFalse("the name is already in use");
70+
message.IsVisible.Should().BeTrue("the user has to be told why OK is disabled");
71+
72+
name.Text = taken + " (2)";
73+
Dispatcher.UIThread.RunJobs();
74+
ok.IsEnabled.Should().BeTrue("the name is free");
75+
message.IsVisible.Should().BeFalse();
76+
}
77+
finally
78+
{
79+
prompt.Close();
80+
dialog.Close();
81+
}
82+
}
83+
84+
[AvaloniaTest]
85+
public void Rename_Accepts_The_Name_The_List_Already_Has()
86+
{
87+
var (dialog, taken) = DialogWithOneList();
88+
// Renaming a list to its own name is a no-op, not a collision with itself.
89+
var prompt = dialog.CreatePrompt("Rename Assembly List", taken, allowedName: taken);
90+
prompt.Show();
91+
try
92+
{
93+
var (_, ok, message) = Controls(prompt);
94+
Dispatcher.UIThread.RunJobs();
95+
ok.IsEnabled.Should().BeTrue();
96+
message.IsVisible.Should().BeFalse();
97+
}
98+
finally
99+
{
100+
prompt.Close();
101+
dialog.Close();
102+
}
103+
}
104+
}

ILSpy/Views/CreateListDialog.axaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
<Grid Margin="12,8" RowDefinitions="Auto,Auto,*,Auto">
99
<TextBlock Grid.Row="0" Text="Enter the name of the list:" Margin="0,0,0,6" />
1010
<TextBox Grid.Row="1" Name="ListNameBox" Margin="0,4" />
11+
<TextBlock Grid.Row="2" Name="NameTakenText" VerticalAlignment="Top"
12+
Foreground="Red" TextWrapping="Wrap" IsVisible="False" />
1113
<StackPanel Grid.Row="3" Orientation="Horizontal" HorizontalAlignment="Right" Spacing="6">
1214
<Button Name="OkButton" Content="OK" IsDefault="True" IsEnabled="False" MinWidth="72" />
1315
<Button Name="CancelButton" Content="Cancel" IsCancel="True" MinWidth="72" />

ILSpy/Views/CreateListDialog.axaml.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
1717
// DEALINGS IN THE SOFTWARE.
1818

19+
using System;
20+
1921
using Avalonia.Controls;
2022
using Avalonia.Markup.Xaml;
2123

@@ -32,25 +34,45 @@ public partial class CreateListDialog : Window
3234
{
3335
TextBox listNameBox = null!;
3436
Button okButton = null!;
37+
TextBlock nameTakenText = null!;
38+
Func<string, bool> isNameTaken = static _ => false;
3539

3640
public CreateListDialog()
3741
{
3842
InitializeComponent();
3943
listNameBox = this.FindControl<TextBox>("ListNameBox")!;
4044
okButton = this.FindControl<Button>("OkButton")!;
41-
listNameBox.TextChanged += (_, _) => okButton.IsEnabled = !string.IsNullOrWhiteSpace(listNameBox.Text);
45+
nameTakenText = this.FindControl<TextBlock>("NameTakenText")!;
46+
nameTakenText.Text = Properties.Resources.ListExistsAlready;
47+
listNameBox.TextChanged += (_, _) => Validate();
4248
okButton.Click += (_, _) => Close(listNameBox.Text);
4349
((Button)this.FindControl<Button>("CancelButton")!).Click += (_, _) => Close(null);
4450
}
4551

46-
public CreateListDialog(string title, string? initialText = null) : this()
52+
/// <param name="isNameTaken">
53+
/// Decides whether the entered name is already in use. Every caller writes into the same
54+
/// set of assembly-list names, so a name that is taken cannot be accepted: the operation
55+
/// behind the prompt would do nothing at all and the dialog would look like it had
56+
/// worked. OK stays disabled for as long as the name collides.
57+
/// </param>
58+
public CreateListDialog(string title, string? initialText = null, Func<string, bool>? isNameTaken = null) : this()
4759
{
4860
Title = title;
61+
this.isNameTaken = isNameTaken ?? this.isNameTaken;
4962
if (!string.IsNullOrEmpty(initialText))
5063
{
5164
listNameBox.Text = initialText;
5265
listNameBox.SelectAll();
5366
}
67+
Validate();
68+
}
69+
70+
void Validate()
71+
{
72+
var name = listNameBox.Text;
73+
bool taken = !string.IsNullOrWhiteSpace(name) && isNameTaken(name);
74+
nameTakenText.IsVisible = taken;
75+
okButton.IsEnabled = !string.IsNullOrWhiteSpace(name) && !taken;
5476
}
5577

5678
void InitializeComponent() => AvaloniaXamlLoader.Load(this);

ILSpy/Views/ManageAssemblyListsDialog.axaml.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,18 @@ void ShowPreconfiguredMenu(Button anchor)
106106
/// <summary>The lists control, exposed for tests to drive the selection.</summary>
107107
internal ListBox ListsControl => listsBox;
108108

109-
async Task<string?> PromptAsync(string title, string? initialText = null)
110-
{
111-
var dlg = new CreateListDialog(title, initialText);
112-
return await dlg.ShowDialog<string?>(this);
113-
}
109+
async Task<string?> PromptAsync(string title, string? initialText = null, string? allowedName = null)
110+
=> await CreatePrompt(title, initialText, allowedName).ShowDialog<string?>(this);
111+
112+
/// <summary>
113+
/// Builds the name prompt for one of the CRUD flows. <paramref name="allowedName"/> is the
114+
/// name the flow may keep - the name of the list being renamed - which is not a collision
115+
/// with itself. Separated from <see cref="PromptAsync"/> so it can be driven by tests
116+
/// without a modal window.
117+
/// </summary>
118+
internal CreateListDialog CreatePrompt(string title, string? initialText = null, string? allowedName = null)
119+
=> new CreateListDialog(title, initialText,
120+
name => name != allowedName && manager.AssemblyLists.Contains(name));
114121

115122
async Task NewListAsync()
116123
{
@@ -135,7 +142,7 @@ async Task RenameListAsync()
135142
{
136143
if (SelectedListName is not { } selected)
137144
return;
138-
var name = await PromptAsync("Rename Assembly List", selected);
145+
var name = await PromptAsync("Rename Assembly List", selected, allowedName: selected);
139146
if (string.IsNullOrWhiteSpace(name) || name == selected || manager.AssemblyLists.Contains(name))
140147
return;
141148
manager.RenameList(selected, name);

0 commit comments

Comments
 (0)