feat(builtin): len, #441 #504
Conversation
| if length <= len(text): return text | ||
| length = len(text) - length | ||
| if length <= len text: return text | ||
| length = len text |
There was a problem hiding this comment.
I am not sure about split those in 2 lines but when the issue is gone I will check that part
|
Now says "Identifier 'len' is a reserved keyword". The first line of code works but the second one no, so @Ph0enixKM maybe there is something to improve in the compiler on parsing a builtin that include another one? |
| @@ -94,6 +94,7 @@ pub fun chars(text: Text): [Text] { | |||
| /// Gets the length of provided text or array. | |||
| #[allow_absurd_cast] | |||
| pub fun len(value): Num { | |||
| echo "The len stdlib is deprecated, use the builtin!" | |||
There was a problem hiding this comment.
We're still in alpha; surely it would be better to remove the standard library function completely, rather than deprecating it?
There was a problem hiding this comment.
We don't have anything to deprecate stuff right now in Amber so I thought that an alert it was the only option
There was a problem hiding this comment.
+1 on this, there is no need to deprecate. we are way too early for that
There was a problem hiding this comment.
this has to be deleted out btw, since len is a reserved keyword.
or renamed. but there is no point in renaming imo because its a breaking change the same way as deleting is
There was a problem hiding this comment.
Yep. I agree with @hdwalters and @b1ek
|
This happens because you have added it to statements modules which means that it works only as a statement like function declaration or loops. What you probably wanted is to add it to an expression modules. Expressions are syntax elements that represent certain value and type. For instance loop is not an expression because it does not represent any type or value. On the other hand a |
There was a problem hiding this comment.
it doesnt work
code:
let a = "1234";
echo len a;
stdout:
bash: line 8: ${#"${__0_a}"}: bad substitution
also it triggers an shfmt error, which was very confusing, i thought its something printed by amber:
<standard input>:2:15: parameter expansion requires a literal
| @@ -162,6 +166,8 @@ impl SyntaxModule<ParserMetadata> for Expr { | |||
| impl TranslateModule for Expr { | |||
| fn translate(&self, meta: &mut TranslateMetadata) -> String { | |||
| translate_expression!(meta, self.value.as_ref().unwrap(), [ | |||
| // Builtin | |||
| Len, | |||
There was a problem hiding this comment.
fyi the order matters in this thing. if its on the bottom it tries to parse that as a VariableGet and throws as error. probably an issue
There was a problem hiding this comment.
In translate this does not matter. This macro expands to a giant match:
match (module) {
...
Len(item) => item.translate()
...
}
It matters only in the parse function above.
|
So we discussed and seems that the issue is the fact that Amber load anyway the stdlib so a @Ph0enixKM says that needs some rewrite Amber in the expression part instead. Also this is the first builtin that return a value compared to the others we already have. |
so push a commit
he should say that himself, or even better push a commit resolving the issue |
Yeah, I am waiting him to know what to do |
|
Sorry for long overdue. I'll add this issue to my priority to-do list |
| @@ -162,6 +166,8 @@ impl SyntaxModule<ParserMetadata> for Expr { | |||
| impl TranslateModule for Expr { | |||
| fn translate(&self, meta: &mut TranslateMetadata) -> String { | |||
| translate_expression!(meta, self.value.as_ref().unwrap(), [ | |||
| // Builtin | |||
| Len, | |||
There was a problem hiding this comment.
In translate this does not matter. This macro expands to a giant match:
match (module) {
...
Len(item) => item.translate()
...
}
It matters only in the parse function above.
|
Closed for #545 |
Fix #441
I am not sure why it is reporting "Variable 'len' does not exist", someone can help me?