Skip to content
This repository has been archived by the owner on May 18, 2023. It is now read-only.

Support ArgumentLists #27

Closed
nex3 opened this issue Nov 7, 2019 · 3 comments · Fixed by #72
Closed

Support ArgumentLists #27

nex3 opened this issue Nov 7, 2019 · 3 comments · Fixed by #72
Labels
enhancement New feature or request

Comments

@nex3
Copy link
Contributor

nex3 commented Nov 7, 2019

The protocol currently mandates that outbound function calls pass variable argument lists "as Value.ArgumentLists", but it doesn't actually define a Value.ArgumentList type. We should do so.

Argument lists pose a particular challenge in that they care about whether their associated keyword arguments were used in practice or not. The compiler throws an error if keyword arguments were passed but not used, since function signatures don't distinguish between taking variable length positional versus keyword arguments, and we want users to have feedback when passing keyword arguments to a function that doesn't accept them.

I think we want to handle this by always passing keyword arguments for each argument list, and adding a repeated int keywords_accessed field to InboundMessage.FunctionCallResponse that indicates which arguments' keywords were accessed in practice. Then the compiler can use this to generate errors where appropriate.

@nex3 nex3 added the bug Something isn't working label Nov 7, 2019
@nex3 nex3 changed the title Doesn't support ArgumentLists Support ArgumentLists Nov 7, 2019
@nex3 nex3 added enhancement New feature or request and removed bug Something isn't working labels Nov 7, 2019
@stof
Copy link
Contributor

stof commented Jan 22, 2021

@nex3 why repeated int keywords_accessed ? I don't see why repeated would be needed. bool keywords_accessed seems enough to me.

@stof
Copy link
Contributor

stof commented Jan 22, 2021

hmm, is it related to the fact that ArgumentList could be passed as a value for a regular argument as well rather than being only related to variable arguments of the current function calls, which could mean we have ArgumentList values elsewhere in the arguments ?

@nex3
Copy link
Contributor Author

nex3 commented Jan 22, 2021

Exactly. A Sass function written in Sass can pass around an argument list as a normal value, including passing it to other functions, which may include custom functions. In all of these cases, we want to track whether the keywords were accessed for each individual ArgumentList object. Admittedly, it's a pretty awkward system, but it seemed like the best triangulation of a number of awkward alternatives.

nex3 added a commit that referenced this issue Aug 5, 2021
nex3 added a commit that referenced this issue Aug 5, 2021
nex3 added a commit that referenced this issue Aug 6, 2021
nex3 added a commit that referenced this issue Aug 6, 2021
nex3 added a commit to sass/dart-sass that referenced this issue Aug 10, 2021
This is necessary to properly forward argument list keywords to the
embedded compiler.

See sass/embedded-protocol#27
nex3 added a commit to sass/dart-sass that referenced this issue Aug 10, 2021
This is necessary to properly forward argument list keywords to the
embedded compiler.

See sass/embedded-protocol#27
@nex3 nex3 closed this as completed in #72 Aug 10, 2021
nex3 added a commit that referenced this issue Aug 10, 2021
nex3 added a commit to sass/dart-sass that referenced this issue Aug 10, 2021
This is necessary to properly forward argument list keywords to the
embedded compiler.

See sass/embedded-protocol#27
nex3 added a commit to sass/dart-sass that referenced this issue Aug 16, 2021
This is necessary to properly forward argument list keywords to the
embedded compiler.

See sass/embedded-protocol#27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants