Skip to content
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

Enhance showPosition() to Accurately Display Error Position by Line Number #2023

Open
satyajitnayk opened this issue Dec 12, 2023 · 13 comments
Labels

Comments

@satyajitnayk
Copy link

satyajitnayk commented Dec 12, 2023

Description

Preserve new-lines in templates when showing the position of parsing-errors.

Current behavior

Input

{{#if}
  HELLO
{{/if}}

Error

Parse error on line 1:
{{#if}  HELLO{{/if}}
-----^

https://handlebarsjs.com/playground.html#format=1&currentExample=%7B%22template%22%3A%22%7B%7B%23if%7D%5Cn%20%20HELLO%5Cn%7B%7B%2Fif%7D%7D%22%2C%22partials%22%3A%5B%5D%2C%22input%22%3A%22%7B%7D%5Cn%22%2C%22output%22%3A%22Yehuda%20KATZ%5Cn%22%2C%22preparationScript%22%3A%22%22%2C%22handlebarsVersion%22%3A%224.7.8%22%7D

Expected behavior

{{#if}
  HELLO
{{/if}}

Error

Parse error on line 1:
  {{#if}
  ----^
    HELLO
  {{/if}}

Original issues

@jaylinski can we enhance showPosition() ??
to Ensures that the ----^ indicator accurately reflects the error position on the relevant line, rather than defaulting to the last line:

For more visit: zaach/jison#406

@jaylinski
Copy link
Member

Can you provide an example on how to reproduce the issue?

@satyajitnayk
Copy link
Author

satyajitnayk commented Dec 13, 2023

In the methods pastInput() & upcomingInput() we are replacing all newline chars with a empty string right!
image

So when I try to remove them & try to get full error template position I am always getting ---^ at last line of template rather than exact error position.
image

My suggestion is that we can improve error message if we could display exact error template instead of converting template to one line.

  1. Current error message thrown: - even though template is multi line error displayed converts it to single line.
    {{#if} HELLO {{/if}}
image
  1. Improved error message: (we can improve error position part to display original template & error position @jaylinski. What's your thought on this?? )
    Parse error on line 1:
    {{#if}
    ----^
    HELLO
    {{/if}}
Expecting 'CLOSE_RAW_BLOCK', 'CLOSE', 'CLOSE_UNESCAPED', 'OPEN_SEXPR', 'CLOSE_SEXPR', 'ID', 'OPEN_BLOCK_PARAMS', 'STRING', 'NUMBER', 'BOOLEAN', 'UNDEFINED', 'NULL', 'DATA', 'SEP', got 'INVALID'

@jaylinski
Copy link
Member

Makes sense. I updated your issue description. Let me know if that is correct.

@satyajitnayk
Copy link
Author

Makes sense. I updated your issue description. Let me know if that is correct.

adjusted expected error message. It looks correct to me @jaylinski

@satyajitnayk
Copy link
Author

One question: Can we modify methods showPosition() , pastInput() ,upcomingInput() OR are these functions are from different package altogether @jaylinski ??

@jaylinski
Copy link
Member

jaylinski commented Dec 15, 2023

Sadly not, the parsing-logic is part of jison (you already opened an issue there) via handlebars-parser.

@satyajitnayk
Copy link
Author

satyajitnayk commented Dec 16, 2023

  • Given that jison is no longer actively maintained(released 3yrs back), relying on it for detailed error handling might not be ideal.

  • The error message is getting truncated/modified inside jison itself so it's practically impossible to get detailed error message later on.

  • My suggestion is to allow users to implement a custom error handler in Handlebars.

    • This allows for flexibility and customisation according to specific needs.
    • Custom Error Handler: Develop a modular error handling system in Handlebars, where users can plug in their custom handlers. This will provide them the freedom to define how detailed or specific they want their error messages to be.
    • Default Handler: Include a default error handler that offers improved messaging as per the current system. This serves users who don't opt for a custom handler.
  • This approach not only solves the issue at hand but also adds a layer of customisation and flexibility that can be beneficial for various use cases.

Thoughts @jaylinski ? 🤔

@jaylinski
Copy link
Member

In order to allow users to implement custom error handlers, one would have to rewrite the current parser.

There are projects that attempt to bring handlebars up-to-date, like https://github.com/nknapp/handlebars-ng. Those are probably a better starting point for such big changes.

@satyajitnayk
Copy link
Author

satyajitnayk commented Dec 17, 2023

I doesn't mean to rewrite whole parser, just send the whole error message as it is from Handlebar(with out truncate/manipulation). Let's users manipulate it as per their need @jaylinski

@jaylinski
Copy link
Member

Yes, directly exposing the lexer object (including the error-message) could probably be done.

@satyajitnayk
Copy link
Author

satyajitnayk commented Dec 20, 2023

Let's do it then @jaylinski .
What is your plan regarding this ?

@jaylinski
Copy link
Member

I currently don't have time to implement this. But I'll be willing to review/accept PRs if somebody improves the error-message (master-branch only).

@satyajitnayk
Copy link
Author

Hi @jaylinski and @nknapp, I've attempted this task but I'm struggling to find a starting point. Could you provide some guidance or a brief overview to help me get started?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants