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

Add test for statement continuation not detected on assignment. #23

Conversation

PierreTechoueyres
Copy link
Collaborator

When an assignment with an statement-continuation occurs below an
if..then the statement continuation is not detected.
This appends with if..then / elsif..then but not with else.

Actually this test fail !!!

When an assignment with an statement-continuation occurs below an
```if..then``` the statement continuation is not detected.
This appends with if..then / elsif..then but not with else.

Actually this test fail !!!
@alex-hhh
Copy link
Owner

alex-hhh commented Jun 2, 2017

Thanks for reporting this. I fixed the original issue and pushed a new set of tests based on the SQL file in this one.

I don't think providing "failing tests" works well with the current framework. In this pull request, you probably generated the syntax file, than manually modified it to match what you expected. Unfortunately, you didn't update the indentation offset, so the test was still failing when the issue was fixed.

For now, I think it is best if problems are just reported with a sample SQL and an explanation, and unit tests are written so that they pass.

Also, the naming convention I used was to use the issue number (or pull request number) as the number for the test. I added some notes to sql-indent-test.el to clarify this.

@alex-hhh alex-hhh closed this Jun 2, 2017
@PierreTechoueyres PierreTechoueyres deleted the pte/pipe-on-assign branch June 6, 2017 21:23
@PierreTechoueyres
Copy link
Collaborator Author

OK, I've tried ... and fail.
I'll try to follow the naming convention for future tests. Thanks for your quick answer.

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.

2 participants