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

Fix for #277 (incorrect line/col number) #288

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

DeanHnter
Copy link

@DeanHnter DeanHnter commented Jan 9, 2025

Fix lpegrex.calcline to Correctly Compute Line and Column Numbers

Functionality Changes:

  • Position Capping: Ensured that the position does not exceed the length of subject by using math.min(position, #subject) instead of an if statement. This prevents potential out-of-bounds errors.
  • Column Number Calculation: Updated the logic to accurately calculate the column number. If the calculated colno is 0, it now correctly assigns position as the column number, ensuring accurate reporting at line boundaries.
  • Line End Handling: Improved the determination of lineend by providing a fallback to #subject + 1 when a newline character is not found. This ensures the entire line is captured correctly, especially for the last line in the subject.

Issue:
Fixes #277 - Incorrect line and line number for syntax error.

WARNING: Please, read this note carefully before submitting a new pull request:

Nelua is open source,
but not very open to contributions in the form of pull requests,
if you would like something fixed or implemented in the core language
try first submitting a bug report or opening a discussion instead of doing a PR.
The authors prefer it this way, so that the ideal solution is always provided,
without unwanted consequences on the project, thus keeping the quality of the software.

If you insist doing a PR, typically for a small bug fix, then follow these guidelines:

  • Make sure the PR description clearly describes the problem and solution. Include the relevant issue number if applicable.
  • Don't send big pull requests (lots of changes), they are difficult to review. It's better to send small pull requests, one at a time.
  • Use different pull requests for different issues, each pull request should only address one issue.
  • When fixing a bug or adding a feature add tests related to your changes to assure the changes will always work as intended in the future and also to cover new lines added.
  • Verify that changes don't break the tests, you can check this with make test.
  • Follow the same coding style rules as the code base.
  • Pull requests just doing style changes are not welcome, a PR must address a real issue.

Motivation

Current version has a very subtle difference in the line number, so its more clear.

Code example

print(

Expected behavior

nelua test.nelua
test.nelua:1:6: syntax error: unclosed parenthesis, did you forget a `)`?

^

Fix `lpegrex.calcline` to Correctly Compute Line and Column Numbers

**Functionality Changes:**
- **Position Capping:** Ensured that the `position` does not exceed the length of `subject` by using `math.min(position, #subject)` instead of an `if` statement. This prevents potential out-of-bounds errors.
- **Column Number Calculation:** Updated the logic to accurately calculate the column number. If the calculated `colno` is `0`, it now correctly assigns `position` as the column number, ensuring accurate reporting at line boundaries.
- **Line End Handling:** Improved the determination of `lineend` by providing a fallback to `#subject + 1` when a newline character is not found. This ensures the entire line is captured correctly, especially for the last line in the `subject`.

**Issue:**
Fixes edubart#277 - Incorrect line and line number for syntax error.
edubart#277
@DeanHnter DeanHnter requested a review from edubart as a code owner January 9, 2025 23:04
@DeanHnter DeanHnter closed this Jan 10, 2025
@DeanHnter DeanHnter reopened this Jan 10, 2025
@DeanHnter DeanHnter closed this Jan 10, 2025
Cleaned the function to using matching instead to handling the multiline issue present in preprocessor test.
@DeanHnter DeanHnter reopened this Jan 10, 2025
@DeanHnter DeanHnter changed the title Fix for #277 (incorrect line and line number) Fix for #277 (incorrect line/col number) Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incorrect line and line number for syntax error
1 participant