-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
chore(remap, external docs): Document compile-time errors #6325
Conversation
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Some smaller questions / suggestions.
docs/reference/remap/errors/106_function_argument_arity_mismatch.cue
Outdated
Show resolved
Hide resolved
docs/reference/remap/errors/107_required_function_argument_missing.cue
Outdated
Show resolved
Hide resolved
docs/reference/remap/errors/108_unknown_function_argument_keyword.cue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet!
Left a few comments, and we'll likely want to update the examples once all of this lands, but this is a great starting point.
docs/reference/remap/errors/108_unknown_function_argument_keyword.cue
Outdated
Show resolved
Hide resolved
+if is_string(.message) { | ||
downcase(.message) | ||
+ } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work yet. I'm not certain if we're tracking this in an issue yet, I believe we are? cc @FungusHumungus.
+string!(.message) | ||
downcase(.message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work, because you still have to assign the result of the string
call to have the compiler deduce the type:
.message = string!(.message)
downcase(.message)
or:
downcase(string!(.message))
While technically the current example could be made to work, I'm hesitant to do so, as it would mean one of two things:
- we'd have to special-case
string
(etc) functions (basically converting them to a function-like built-in) to allow manipulating compiler type state, or - allow functions in general to manipulate type state, which I suspect would be a recipe for breaking core language safety guarantees by loading a badly implemented function.
Co-authored-by: Pablo Sichert <[email protected]>
Signed-off-by: binarylogic <[email protected]>
…ch.cue Co-authored-by: Pablo Sichert <[email protected]>
…sing.cue Co-authored-by: Pablo Sichert <[email protected]>
…ord.cue Co-authored-by: Pablo Sichert <[email protected]>
Co-authored-by: Pablo Sichert <[email protected]>
Co-authored-by: Pablo Sichert <[email protected]>
…ord.cue Co-authored-by: Jean Mertz <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Thanks for the reviews. We will circle back and update the examples before launching. |
Signed-off-by: binarylogic [email protected]