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

Deprecate variadic arguments to macro #raise #13394

Open
Blacksmoke16 opened this issue Apr 26, 2023 · 3 comments
Open

Deprecate variadic arguments to macro #raise #13394

Blacksmoke16 opened this issue Apr 26, 2023 · 3 comments

Comments

@Blacksmoke16
Copy link
Member

Wanted to create this issue before opening a PR.

Currently you can call macro raise with 0 or more arguments:

{%
  # No arguments
  raise

  # Multiple arguments
  raise 10, "foo", false
%}

Additionally you can pass whatever kind of argument as well. I think it would make sense to deprecate the ability to call it with 0, or more than 1 argument. We can raise a warning in the meantime.

We could also go a step further and require the argument to be a StringLitereal, raising a separate warning for that if we so desire.

Originally posted by @Blacksmoke16 in #13262 (comment)

@straight-shoota
Copy link
Member

Sounds good. I don't see a relevant use case for multiple arguments. Runtime raise doesn't have that either.
Although this behaviour was implemented from the beginning: 9e26cf9
@asterite Do you have any recollection why multiple arguments are allowed and joined?

Not sure if a deprecation warning gets much attention in the case of a macro raise, but we can try it =)

@Blacksmoke16
Copy link
Member Author

And what about the implicit string casting? I.e. should we also enforce you pass only strings to it or is to_s whatever you pass it expected?

@straight-shoota
Copy link
Member

I guess implicit casting shouldn't really be necessary. We can probably drop it. Not sure if it matters much, though.
Same goes for the variadic arguments: There's no problem with having this feature. Removing it is just for cleaning up a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants