-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
Add version of ListViewItemCollection.AddRange() which accepts IList. #10924
Comments
Edit: I think changing the overload to |
…istViewItem>. Fixes dotnet#10924.
I made an experimental commit that should resolve this issue for .NET 9+: AraHaan@5a4d314 Things to note: While it won't be breaking runtime wise, it might force people to need to recompile their code if they use the version of the overload that accepts |
Your test should use |
@AraHaan can you please update the issue description with |
Sure |
Changed it just now: AraHaan@7e4ac14 |
@JeremyKuhne is actively looking at this and we agree it's a great idea. He's going to create a PR that addresses this space more broadly. Stay tuned, we will be taking a change here in .NET 9 🎉 |
So basically the changes I did will be in their PR or should I PR my changes first since they are already completed on this at least to save some of their time? |
@AraHaan I already wrote the code, I should have it up tomorrow. It implements the |
Ugh. This still has ambiguity after adding the public class ListView.ListViewItemCollection : IList
{
public void AddRange(params ListViewItem[] items);
public void AddRange(ListViewItemCollection items);
} ListView listView = new();
List<ListViewItem> sourceEntries = [];
// This gives a suggestion for
listView.Items.AddRange(sourceEntries.ToArray());
// this, but this is ambiguous
// listView.Items.AddRange([.. sourceEntries]); |
Have the standalone repro now: using System;
using System.Collections;
using System.Collections.Generic;
public class ListViewItem
{
}
public class ListViewItemCollection : IList
{
public object? this[int index] { get => default; set{}}
public bool IsFixedSize => default;
public bool IsReadOnly => default;
public int Count => default;
public bool IsSynchronized => default;
public object SyncRoot => this;
public int Add(object? value) => default;
public void AddRange(params ListViewItem[] items) {}
public void AddRange(ListViewItemCollection items) {}
public void Clear() {}
public bool Contains(object? value) => default;
public void CopyTo(Array array, int index) {}
public IEnumerator GetEnumerator() => default!;
public int IndexOf(object? value) => default;
public void Insert(int index, object? value) {}
public void Remove(object? value) {}
public void RemoveAt(int index) {}
}
public class Test
{
public void M()
{
ListViewItemCollection listViewCollection = new();
List<ListViewItem> sourceEntries = [];
listViewCollection.AddRange(sourceEntries.ToArray());
listViewCollection.AddRange([.. sourceEntries]);
}
} |
Ok, there is no great way to solve this completely transparently that doesn't make things worse. We could add an implicit conversion: public static implicit operator ListViewItemCollection(ListViewItem[] arr)
{
var col = new ListViewItemCollection();
col.AddRange(arr);
return col;
} But that adds additional overhead. If we add |
Would this just be solved by .NET itself supporting ranges by dotnet/runtime#18087? |
Background and motivation
Currently when I call
ToArray()
on anList<ListViewItem>
to then add the items in bulk in this way:This then generates a warning for the usage of
ToArray()
saying the inititialization can be simplified to:Which compiles in the IDE, however when doing command line compiles via
dotnet build -c Release
this results in:As such a better option would be to add a version of
AddRange
that acceptsList<ListViewItem>
would be a great addition to the API and help fix this problem as well.API Proposal
API Usage
Alternative Designs
Change the existing
ListViewItemCollection
overload ofAddRange
to also include all types that inherit fromIList
likeList<T>
for example.Risks
Minimal, since it deals with adding ListViewItems from a
List<ListViewItem>
type of collection.Will this feature affect UI controls?
I think this would minimally affect them.
The text was updated successfully, but these errors were encountered: