') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Add LINQ APIs for Index and Range (#28776) by Dixin · Pull Request #47950 · dotnet/runtime · GitHub
Skip to content

Add LINQ APIs for Index and Range (#28776) - #47950

Closed
Dixin wants to merge 0 commit into
dotnet:masterfrom
Dixin:linq-index-range
Closed

Add LINQ APIs for Index and Range (#28776)#47950
Dixin wants to merge 0 commit into
dotnet:masterfrom
Dixin:linq-index-range

Conversation

@Dixin

@DixinDixin commented Feb 6, 2021

Copy link
Copy Markdown
Contributor

Add LINQ APIs for index and range:

  • Enumerable
    • ElementAt(Index)
    • ElementAtOrDefault(Index)
    • Take(Range)
  • Queryable
    • ElementAt(Index)
    • ElementAtOrDefault(Index)
    • Take(Range)

Close#28776.

@ghost

ghost commented Feb 6, 2021

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost

ghost commented Feb 6, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

Add LINQ APIs for index and range:

  • Enumerable
    • ElementAt(Index)
    • ElementAtOrDefault(Index)
    • Take(Range)
  • Queryable
    • ElementAt(Index)
    • ElementAtOrDefault(Index)
    • Take(Range)

Close #28776.

Author:Dixin
Assignees:-
Labels:

area-System.Linq, new-api-needs-documentation

Milestone:-

stephentoub
stephentoub reviewed Feb 6, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
stephentoub
stephentoub reviewed Feb 6, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
stephentoub
stephentoub reviewed Feb 6, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 6, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 6, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 6, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
stephentoub
stephentoub reviewed Feb 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
stephentoub
stephentoub reviewed Feb 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
stephentoub
stephentoub reviewed Feb 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
stephentoub
stephentoub reviewed Feb 8, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
stephentoub
stephentoub reviewed Feb 8, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
@runfoapprunfoappBot mentioned this pull request Feb 8, 2021
Closed
eiriktsarpalis
eiriktsarpalis reviewed Feb 8, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 8, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 8, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 8, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/tests/ElementAtOrDefaultTests.cs Outdated
select x;

Assert.Equal(q.ElementAtOrDefault(3), q.ElementAtOrDefault(3));
var q = Repeat(_ => from x in new[] { 9999, 0, 888, -1, 66, -777, 1, 2, -12345 }

@eiriktsarpaliseiriktsarpalisFeb 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious what the motivation for this method is. It took me a while to grasp but it seems to be creating 3 identical IEnumerable instances? Why would that be needed?

@DixinDixinFeb 8, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The original unit test creates a source, then calls ElementAtOrDefault twice on the same source, and asserts the result.

I am keeping the consistency.

@eiriktsarpaliseiriktsarpalisFeb 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, but for all intents and purposes the source enumerable in the test is immutable. We should not expect noticeable difference no matter how many times it has been enumerated. I would recommend keeping the tests as simple as possible (in this case and most other cases using the Repeat method, q can be enumerated as many times as needed. Assuming this were not the case, perhaps MemberData is preferable?

@eiriktsarpaliseiriktsarpalis mentioned this pull request Feb 9, 2021
Closed
6 tasks
@runfoapprunfoappBot mentioned this pull request Feb 15, 2021
Closed
eiriktsarpalis
eiriktsarpalis reviewed Feb 15, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 15, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 15, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 17, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 18, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/tests/ElementAtTests.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 18, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/tests/ElementAtTests.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 18, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/src/System/Linq/ElementAt.cs Outdated
eiriktsarpalis
eiriktsarpalis reviewed Feb 19, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/tests/TakeTests.cs Outdated
var en1 = iterator1 as IEnumerator<int>;
Assert.False(en1 != null && en1.MoveNext());

var iterator2 = NumberRangeGuaranteedNotCollectionType(0, 3).Take(2);

@eiriktsarpaliseiriktsarpalisFeb 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this code duplicated?

@DixinDixinFeb 20, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed.

eiriktsarpalis
eiriktsarpalis reviewed Feb 19, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/tests/TakeTests.cs Outdated
var en1 = iterator1 as IEnumerator<int>;
Assert.False(en1 != null && en1.MoveNext());

var iterator2 = NumberRangeGuaranteedNotCollectionType(0, 3).ToList().Take(2);

@eiriktsarpaliseiriktsarpalisFeb 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

@DixinDixinFeb 20, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed.

eiriktsarpalis
eiriktsarpalis reviewed Feb 19, 2021
View reviewed changes
Comment threadsrc/libraries/System.Linq/tests/TakeTests.cs Outdated

var source = new DelegateIterator<int>(
moveNext: () => ++state <= sourceCount,
var source = Repeat(index => new DelegateIterator<int>(

@eiriktsarpaliseiriktsarpalisFeb 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the only test I could find where Repeat is really needed.

Nevertheless the test has too many moving parts and each section is applying different sets of assertions. I would recommend either breaking this up into separate unit tests or perhaps it might be possible to factor out some of the state management into a local method.

@DixinDixinFeb 20, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Refactored the code.

eiriktsarpalis
eiriktsarpalis approved these changes Feb 19, 2021
View reviewed changes

@eiriktsarpaliseiriktsarpalis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, pending some feedback in the test code.

@DixinDixin closed this Feb 20, 2021
@stephentoub

stephentoub commented Feb 20, 2021

Copy link
Copy Markdown
Member

Did you intend to close this? Seemed almost ready to merge.

@Dixin

Dixin commented Feb 20, 2021

Copy link
Copy Markdown
ContributorAuthor

@stephentoub No. Something went wrong when I manipulate my local git repo and push. Can you please reopen it?

@stephentoub

stephentoub commented Feb 20, 2021

Copy link
Copy Markdown
Member

GitHub won't let me. I think the branch must have gotten messed up. Can you open a new PR?

@DixinDixin mentioned this pull request Feb 20, 2021
Merged
@Dixin

Dixin commented Feb 20, 2021•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@stephentoub@eiriktsarpalis Please use PR #48559.

I think the branch must have gotten messed up.

Yes. I did git reset --hard xxx & git push -f on an incorrect branch with an incorrect hash code xxx. It caused GitHub to auto close this PR. Sorry for the inconvenience.

@stephentoub

stephentoub commented Feb 21, 2021

Copy link
Copy Markdown
Member

No worries. Thanks.

@ghostghost locked as resolved and limited conversation to collaborators Mar 24, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub left review comments
@eiriktsarpaliseiriktsarpaliseiriktsarpalis approved these changes

Assignees

No one assigned

Labels

area-System.Linqnew-api-needs-documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Proposal (& implementation): LINQ APIs to enable C# 8.0 index and range for IEnumerable<T>

3 participants

@Dixin@stephentoub@eiriktsarpalis