This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
feat(rome_rowan): implement DoubleEndedIterator for node lists
#2672
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying with
|
| Latest commit: |
4120e45
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://70f0004b.tools-8rn.pages.dev |
| Branch Preview URL: | https://feature-slots-as-double-ende.tools-8rn.pages.dev |
|
!bench_formatter |
Formatter Benchmark Results |
|
!bench_formatter |
Formatter Benchmark Results |
MichaReiser
suggested changes
Jun 8, 2022
|
Not sure we are implementing the fn main() {
let list = vec![1,2,3,4];
let mut iter = list.iter();
println!("{:?}", iter.next());
println!("{:?}", iter.next_back());
println!("{:?}", iter.next());
println!("{:?}", iter.next_back());
println!("{:?}", iter.next());
println!("{:?}", iter.next_back());
}outputs This happens because:
I tried the following test and failed: #[test]
fn double_iterator_meet_at_middle() {
let list = build_list(vec![
(Some(1), Some(",")),
(Some(2), Some(",")),
(Some(3), Some(",")),
(Some(4), None),
]);
let mut iter = list.elements();
let element = iter.next().unwrap();
assert_eq!(element.node().unwrap().text(), "1");
let element = iter.next_back().unwrap();
assert_eq!(element.node().unwrap().text(), "4");
let element = iter.next().unwrap();
assert_eq!(element.node().unwrap().text(), "2");
let element = iter.next_back().unwrap();
assert_eq!(element.node().unwrap().text(), "3");
assert!(iter.next().is_none()); // line 701
assert!(iter.next_back().is_none());
}and failed with: |
Damn I missed that part! Thank you for looking into this! |
Parser conformance results on ubuntu-latestjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
|
Please re-read the description, there's more context now |
MichaReiser
reviewed
Jun 9, 2022
ematipico
force-pushed
the
feature/slots-as-double-ended-iterators
branch
from
3c17eb7
to
2339019
Compare
ematipico
force-pushed
the
feature/slots-as-double-ended-iterators
branch
from
78ab67c
to
a9d1923
Compare
|
@MichaReiser I addressed your concerns, your help was really helpful, thank you! Now the code should be clear and better to read. I believe we are ready to ship it! |
ematipico
force-pushed
the
feature/slots-as-double-ended-iterators
branch
from
a9d1923
to
4120e45
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds the ability to consume an
AstSeparatedListfrom the back by implementingDoubleEndedIterator.The reason why I implemented this is because there have been cases where I needed to retrieve the last elements of a list. This will be useful, for example, inside the formatter when implementing the formatting of call arguments.
Checking a separated list from the back adds a bit more of complexity because the last separator is not mandatory, which means that we should handle cases where this is not an error.
In order to achieve that, I had to make the slots peekable, because sometimes we don't want to consume two slots. An example is when we have
[1, separator, 3, separator, 5]. We want to consume only5in the first iteration, but then we want to consumeseparatorand3.I added the
unreacheablemacro. If we hit that macro, it means that are edge cases we have to handle.Test Plan
Created new tests to cover the cases:
Noneis returnedassert_rev_elements, we callrevon all vectors (revneedsnext_back), so we should cover all the edge cases that were previously handled.