Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bracket list expression for initialization without escaping #2003

Merged
merged 10 commits into from
Sep 24, 2023

Conversation

oowekyala
Copy link
Collaborator

Add an expression kind for Python and TS lists, and Rust arrays.

Fix #1981

This requires an update to the website.

@lhstrh
Copy link
Member

lhstrh commented Sep 13, 2023

Fantastic! This is really nice, @oowekyala. It occurs to me that even if we were to aim for a clearer separation between target code and LF code, initializers are probably the places where it would remain to pay off to elegantly mix languages (i.e., without escaping)...

@lhstrh
Copy link
Member

lhstrh commented Sep 14, 2023

Looks like our build-epoch workflow doesn't deal well with forks. It's failing because it is attempting to resolve the ref clem.bracket-list-expr that corresponds to this PR in the lf-lang repo instead of your fork... I guess the workflow should take as input a string that identifies the fork and then just replace the lingua-franca submodule before checking out the given ref...

@lhstrh lhstrh force-pushed the clem.bracket-list-expr branch 4 times, most recently from 371b44c to efb8f55 Compare September 15, 2023 01:16
@lhstrh lhstrh force-pushed the clem.bracket-list-expr branch from efb8f55 to 91b219b Compare September 15, 2023 01:24
@lhstrh
Copy link
Member

lhstrh commented Sep 15, 2023

Looks like our build-epoch workflow doesn't deal well with forks. It's failing because it is attempting to resolve the ref clem.bracket-list-expr that corresponds to this PR in the lf-lang repo instead of your fork... I guess the workflow should take as input a string that identifies the fork and then just replace the lingua-franca submodule before checking out the given ref...

OK, fixed this!

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@oowekyala
Copy link
Collaborator Author

Thanks Marten for fixing the CI!

@oowekyala oowekyala marked this pull request as ready for review September 18, 2023 14:50
@lhstrh lhstrh changed the title Add bracket list expression Bracket list expression for initialization without escaping Sep 18, 2023
@lhstrh lhstrh added the feature New feature label Sep 18, 2023
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Looks great!

@lhstrh lhstrh added this pull request to the merge queue Sep 18, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 18, 2023
cmnrd
cmnrd previously requested changes Sep 19, 2023
Copy link
Collaborator

@cmnrd cmnrd left a comment

Choose a reason for hiding this comment

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

This looks great! The only request I have, is that more Python tests should be updated. For instance ArrayAsParamenter.lf still uses the old syntax. There might be more.

This is beyond this PR, but we should soon remove the old name(<expr>*) syntax (#1675).

@lhstrh lhstrh dismissed cmnrd’s stale review September 22, 2023 23:15

Fixed the test you mentioned; didn't find any others.

@lhstrh lhstrh enabled auto-merge September 22, 2023 23:15
@lhstrh lhstrh added this pull request to the merge queue Sep 24, 2023
Merged via the queue into lf-lang:master with commit c623210 Sep 24, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "square bracket list literal" to grammar
3 participants