elixir
elixir copied to clipboard
elixir-lang
Improve mismatched delimiter error message
Stealing from Rust:
error: mismatched closing delimiter: `)`
--> /Users/jose/OSS/explorer/native/explorer/src/encoding.rs:120:12
|
120 | unsafe { Term::new(env, list)) }
| ^ unclosed delimiter ^ mismatched closing delimiter
I am thinking about that improvement, and I have a question about short lines. In this example foo(() make sense has an indicator to column on top, like above?
| ˇ another error here
1 | foo(()
| ˆ random error here
Good point. I would probably always have the opening one on top and the closing one on the bottom.
@josevalim that example makes sense for the case?
** (TokenMissingError) nofile:1:7: missing terminator: ) (for "(" starting at line 1)
| ˇ unclosed delimiter
1 | foo(()
| ˆ missing delimiter
I am working under the currently mismatching error message, but with the example about Rust, we don't have a similar case.
** (SyntaxError) nofile:1:8: unexpected token: )
|
1 | foo(()))
| ˆ
Missing delimiter is tricky because if item happens at very distinct lines. I would rather focus on the mismatched ones. We may need to add an special exception for them.
@josevalim I made some tests using the "end" delimiter, and my results were this, looks good? If it is good I will create a PR with my changes and you can follow my changes in other clauses too.
defmodule MyApp do
def one do
end
def two
end
end
nofile:6:2: mismatched closing delimiter: end
HINT: it looks like the "end" on line 6 does not have a matching "do" defined before it
|
6 | end
| ^ mismatched delimiter
@lucassouzamatos this issue is not about the case you brought up because in your case we got a token when none was expected. This issue is about the case where we expected one and we got another! :)
@josevalim of course, i did not notice that, I will follow on this side. But, it is a good improvement adding the snippet in that cases like I specified? Maybe for other PR? Or can I just ignore it?
I am not sure because in those cases we are just guessing the error is there and we could be pointing at the wrong thing.
Hey @josevalim, what do you think the diagnostic we would print for this should look like?
@viniciusmuller i think for this one we will need to show a very specific snippet/diagnostic without reusing the existing infrastructure.
And about the visuals, should we strive for something like the snippet in this PR?
Nice to note that there are two cases here:
- We have start and end delimiter in the same line, that's ok
- A case where we have different start and end lines, where I think we would like to read both and display them, with a marker in each
In our implementation, this is what we currently have:
- same line
** (SyntaxError) invalid syntax found on unclosed.ex:2:14:
error: unexpected token: ]
HINT: the "{" on line 2 is missing terminator "}"
│
2 │ {10, 20, 30]
│ ^
│
└─ unclosed.ex:2:14
(elixir 1.16.0-dev) lib/code.ex:1463: Code.require_file/2
^ Not entirely related, but I think we could remove those indentations (HINT:) in some messages now that we format differently
- different lines
** (SyntaxError) invalid syntax found on unclosed.ex:3:5:
error: unexpected token: ]
HINT: the "{" on line 2 is missing terminator "}"
│
3 │ ]
│ ^
│
└─ unclosed.ex:3:5
(elixir 1.16.0-dev) lib/code.ex:1463: Code.require_file/2
That's how rust does it currently:
- same line
error: mismatched closing delimiter: `]`
--> src/main.rs:2:13
|
2 | let a = { 1, 2, 3 ]
| ^ ^ mismatched closing delimiter
| |
| unclosed delimiter
- different lines
error: mismatched closing delimiter: `]`
--> src/main.rs:1:11
|
1 | fn main() {
| ^ unclosed delimiter
2 | println!("Hello, world!");
3 | ]
| ^ mismatched closing delimiter
- Line too long
error: mismatched closing delimiter: `]`
--> src/main.rs:2:133
|
2 | ...ongnameverylongnameverylongnameverylongname = { 1, 2, 3 ];
| ^ ^ mismatched closing delimiter
| |
| unclosed delimiter
Do you think we could follow this format? The structure is already pretty similar to what we have
I'll take a better look at how we can implement this, but I believe it would require adding the end and start lines for the delimiters to the exception and figure out a way to check if it was a matched delimiter error, and then call a new function that formats this snippet that formats things like this (I believe in the future could be used for unclosed "do"s or strings)
WDYT?
What happens when the mismatch is after 100 lines… so we show all 100 lines or does Rust collapse?
Also keep in mind unclosed terminator is different from mismatched one. This one is about the latter and I think we should keep the discussions distinct (although we can improve the unclosed one separately if we desire).
In any case, your proposals look great to me. :)
Also, don’t necessarily try to use the existing diagnostic infrastructure. It may be easier to implement this in Elixir and specific to this exception. :)
What happens when the mismatch is after 100 lines… so we show all 100 lines or does Rust collapse?
It collapses, showing the first and last line in the range
error: mismatched closing delimiter: `]`
--> src/main.rs:1:11
|
1 | fn main() {
| ^ unclosed delimiter
...
13 | ]
| ^ mismatched closing delimiter
Also keep in mind unclosed terminator is different from mismatched one
No problems, I meant having an abstraction that we can later just refactor and add this case if it makes sense (previously we stopped showing the snippet on these cases because the parser would give us the last line, but if we show a range from the start to the last it might make more sense)
Also, don’t necessarily try to use the existing diagnostic infrastructure
I'll take a look, I think it should be doable in elixir as there shouldn't be as much code we would need to reuse