support todo in constants
#5496
Replies: 5 comments 2 replies
-
|
I'd expect that as well, it seems the most intuitive way to go about it to me. This seems quite straightforward on the Erlang target. The following pub const wibble = Thing(field: todo)would just become {thing, erlang:error(#{gleam_error => todo,
message => <<"`todo` expression evaluated. This code has not yet been implemented."/utf8>>,
file => <<?FILEPATH/utf8>>,
module => <<"main"/utf8>>,
function => <<"main"/utf8>>,
line => 9})}wherever the constant is used. But on JavaScript, where top-level module constants are produced, the current strategy of a lambda that's immediately called to throw an exception means we'd throw when the constant is evaluated, not when it's used. For the JS target, we could replace the entire constant with the |
Beta Was this translation helpful? Give feedback.
-
|
Thinking a bit more about this, I can think of a couple of things that can be tricky and we should consider. Since constants are handled slightly differently on JS and Erlang, introducing a side-effecting expression like
So far this hasn't been an issue since we limit constants to be pure, but this would introduce some strange differences I'm afraid. I'm not sure that would be acceptable |
Beta Was this translation helpful? Give feedback.
-
|
Oh haha, we've thought of the same thing at the same time! |
Beta Was this translation helpful? Give feedback.
-
|
Do we want to have gleam program with todo in constant to even be runnable ? Adding todo in constant make only sense for writing in LSP with code actions but contrary to our current todo, this one can’t really be a warning since gleam constant are supposed to be compile time know (literal) What I want to say with this is having a todo inside a constant should not emit a warning but emit an error because that’s something that need to be fixed ASAP however it should be a recognized keyword for formater and LSP to work (specially for the code action of #4730). So it’s not really necessary to think what a JS or Erlang output could be since we would just forbid the build. Like when we use a non literal value in a case. |
Beta Was this translation helpful? Give feedback.
-
|
@lpil I’m not sure to have understood. What are the reasons to transform this github issue into a github conversation. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
I'd like to discuss what supporting
todoin constants would look like. Right now it's a syntax error, but allowing them could have its use, for example enabling the "Fill labels" code action in constant records: #4730What would its behaviour be? Rendering the exception throwing code where the constant is used sounds reasonable to me
Beta Was this translation helpful? Give feedback.
All reactions