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

Named parameters #141

Merged
merged 8 commits into from
Dec 2, 2024
Merged

Named parameters #141

merged 8 commits into from
Dec 2, 2024

Conversation

nezuo
Copy link
Contributor

@nezuo nezuo commented Nov 30, 2024

TODO:

  • Docs

@sasial-dev sasial-dev self-assigned this Nov 30, 2024
@sasial-dev sasial-dev marked this pull request as draft November 30, 2024 04:17
@nezuo nezuo marked this pull request as ready for review November 30, 2024 22:25
Copy link
Collaborator

@sasial-dev sasial-dev left a comment

Choose a reason for hiding this comment

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

Looking good! Got a few initial questions

zap/src/output/luau/client.rs Show resolved Hide resolved
zap/src/parser/convert.rs Show resolved Hide resolved
zap/src/parser/grammar.lalrpop Outdated Show resolved Hide resolved
@nezuo
Copy link
Contributor Author

nezuo commented Dec 1, 2024

Added error for duplicate parameters:
image


### `rets`

This field determines the data that is sent back to the client from the server. It can be any [Zap type](./types.md).

- If the server doesn't return any data, the `rets` field should be excluded.
- Unlike `args`, `rets` cannot be named.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we need a better error (sorry) then

error[2002]: expected ")", ",", "?", "["
  ┌─ net.zap:4:16
  │
4 │     rets: (test: enum { Success, Fail }, bar: string)
  │                ^ unexpected token

Copy link
Collaborator

@sasial-dev sasial-dev left a comment

Choose a reason for hiding this comment

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

Other than the comment above - LGTM

@nezuo
Copy link
Contributor Author

nezuo commented Dec 1, 2024

Improved error for named rets:

error[3016]: rets cannot be named
   ┌─ net.zap:88:1288rets: (foo: f64, boolean),
   │            ^^^ must be removed

@sasial-dev
Copy link
Collaborator

Thank you!

@sasial-dev sasial-dev merged commit 1f9a77e into red-blox:0.6.x Dec 2, 2024
2 checks passed
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.

2 participants