-
Notifications
You must be signed in to change notification settings - Fork 41
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
Improve error message #93
Comments
Yeah definitely something we need to improve! I'm almost done with a major refactor of tags and custom tags so I would avoid any big change on the parser for now. Hoping to get this out tomorrow! |
For the error messages we can probably play with https://hexdocs.pm/nimble_parsec/NimbleParsec.html#label/3 ? I've never used it but I hope it can help us? For the file and line case: Maybe the Line 44 in 157d799
So maybe we could change it to use |
I have played with line and offset from the error return by |
I have a solution but it's quite cumbersome. I keep parsing the remaining template until error line is 1. then I got the exact blog which cause bug. Something like this def parse(text, opts \\ []) do
parser = Keyword.get(opts, :parser, Solid.Parser)
template_file = Keyword.get(opts, :template)
case parser.parse(text) do
{:ok, result, _, _, _, _} ->
{:ok, %Template{parsed_template: result}}
{:error, reason, remaining, _, {line, _}, _} ->
{line, reason, remaining} =
Enum.reduce_while(1..100, {line, reason, remaining}, fn _, {line, _, remaining} ->
case parser.parse(remaining) do
{:error, reason, remaining, _, {l, _}, _} ->
if l == 1 do
{:halt, {line + l, reason, remaining}}
else
{:cont, {line + l, reason, remaining}}
end
_ ->
{:halt, {0, nil, ""}}
end
end)
[template | _] = String.split(remaining, "\n", parts: 2)
{:error, TemplateError.exception([reason, template_file, line, template])}
end
end |
Interesting! It might be a good start then? Until we find a better solution I think this is better than what we have today. What do you think? |
Sorry for slow response, I will create a PR for you to play with |
after do more testing with this idea, for some cases it points to correct line which cause error, some other cases it's still wrong. So I think that we should try other solutions |
Currently if parsing failed, solid the return message
It does not provide much detail so you don't know where to fix or what to fix.
It will be better if the error message can provide more details:
For example
and the line which error return is line 1, not line 3
The text was updated successfully, but these errors were encountered: