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

Line numbers might be incorrect in LiquidSyntaxError for multi-line templates #162

Closed
nasudadada opened this issue Dec 25, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@nasudadada
Copy link

Hi there!

I think I've found an issue with line number reporting in error messages, so I wanted to check if this is expected behavior.

Possible issue: Line numbers in LiquidSyntaxError might be incorrect

Description

I noticed that in some cases with multi-line templates, the line number in LiquidSyntaxError messages might not be showing the actual line where the error occurred.

Steps to Reproduce

from liquid import Template

template_string = """Hello, 
My name is {{{ name }}
"""

try:
    Template(template_string)
except LiquidSyntaxError as e:
    print(f"Error message: {e}")

Expected Behavior

I would expect the error message to show line 2, as that's where the syntax error appears to be.

Actual Behavior

The error message seems to always show line 1 in this case.

Investigation

I've been looking at the error handling in environment.py, and I wonder if this part might be related:

if not isinstance(exc, Error):
    exc = exc(msg, linenum=linenum)
elif not exc.linenum:
    exc.linenum = linenum

Questions

  1. Is this the intended behavior?
  2. If not, would it make sense to update the line number handling?
  3. Are there any test cases that specifically check for correct line numbers in syntax errors?

Additional Context

  • I noticed that some test cases in test_malformed.py do handle line numbers correctly, so I might be missing something about how this should work.

I'd be happy to investigate further or help with a fix if this turns out to be a bug. Please let me know if you'd like me to provide any additional information or create a PR.

Thank you for maintaining this great library!

@jg-rp
Copy link
Owner

jg-rp commented Dec 25, 2024

Hi @nasudadada,

Thanks for reporting this. It is not intended behaviour. I was failing to pass a line number to some expression parsing functions in multiple place, causing the line number to default to 1.

It still needs a few more tests, but PR #163 should fix this.

@nasudadada
Copy link
Author

Hi @jg-rp

Thank you for the quick response and for already working on the fix!
Really appreciate your prompt attention to this issue.

@jg-rp
Copy link
Owner

jg-rp commented Dec 26, 2024

This should be fixed in version 1.12.2, now released.

I've been working on improving exceptions and error messages in Python Liquid2 (separate repository). Using the example above, we get the following error message from Python Liquid2.

from liquid2 import render

source = """\
Hello,
My name is {{{ name }}
"""

render(source, name="Bob")
liquid2.exceptions.LiquidSyntaxError: unexpected '{'
  -> 'My name is {{{ name }}' 2:13
  |
2 | My name is {{{ name }}
  |              ^ unexpected '{'

@nasudadada
Copy link
Author

Thank you so much for the quick fix and release of version 1.12.2!

I've confirmed that the issue has been resolved in the new version.

Thanks for maintaining such a great library!

@jg-rp jg-rp closed this as completed Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants