Skip to content

add use-path grammar#245

Merged
lukewagner merged 3 commits intoWebAssembly:mainfrom
cardoso:patch-2
Sep 13, 2023
Merged

add use-path grammar#245
lukewagner merged 3 commits intoWebAssembly:mainfrom
cardoso:patch-2

Conversation

@cardoso
Copy link
Contributor

@cardoso cardoso commented Sep 11, 2023

This seems to be missing from #174 . I just took a rough look at wit-parser and this seems to be what it's doing currently.

cc @Mossaka @alexcrichton

Copy link
Contributor

@Mossaka Mossaka left a comment

Choose a reason for hiding this comment

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

Thanks! Lgtm

Copy link
Collaborator

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks! Could use-path reuse the preexisting interface production perhaps? That may require renaming interface, however, since include refers to worlds instead of interfaces.

@cardoso
Copy link
Contributor Author

cardoso commented Sep 11, 2023

@alexcrichton can it take the version as well? The parser accepts it, but I didn't see ui tests for it. If the parser is right, I can improve this and add more tests there.

@alexcrichton
Copy link
Collaborator

It can, yeah, and would be good to add tests as well!

@cardoso
Copy link
Contributor Author

cardoso commented Sep 11, 2023

Okay, so it seems they're the same:

interface ::= id
            | id ':' id '/' id ('@' valid-semver)?

If this is it, then I'll open the PR for the parser once this is merged.

@cardoso
Copy link
Contributor Author

cardoso commented Sep 11, 2023

It does seem confusing though since it should refer to worlds, but I'll leave it to you.

@cardoso
Copy link
Contributor Author

cardoso commented Sep 13, 2023

Thanks @alexcrichton . I think it reads better now (and uses the same naming as wit-parser).

@lukewagner
Copy link
Member

Thanks!

@lukewagner lukewagner merged commit 080845b into WebAssembly:main Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants