Skip to content
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

Introduce support to range iterations and SFR tags #685

Merged
merged 2 commits into from
Dec 14, 2022
Merged

Conversation

karreiro
Copy link
Contributor

@karreiro karreiro commented Dec 14, 2022

Fixes #681, #682, and #684
(I've opened a single PR because the fixes intersect a bit in the code, and I'd have dependent PRs)


 - Introduce support to range iterations
 - `AssignmentsCompletionProvider` no longer crashes when it finds an
   unexpected lookup
Copy link
Contributor

@Poitrin Poitrin left a comment

Choose a reason for hiding this comment

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

Impressive work! 😮

I just found one little bug in this scenario:

{%- assign number_of_slides = 4 %}
{%- for i in (1..number_of_slides) -%}
    <div id="Slide-{{ block.

14-59-nilsx-xfe2v

You even thought of this case in test_assignments_finder_with_for_statements_and_complex_ranges, but, alas, the range starts with a literal number and ends with a variable x)

@karreiro
Copy link
Contributor Author

Thank you, @Poitrin! Excellent catch 🚀

I've applied the fix in a new commit :)

Copy link
Contributor

@Poitrin Poitrin left a comment

Choose a reason for hiding this comment

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

Thanks for your changes, LGTM!

Copy link
Contributor

@mgmanzella mgmanzella left a comment

Choose a reason for hiding this comment

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

🚀

@karreiro karreiro merged commit 2328269 into main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants