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

Formatting comments, new lines are not kept #50

Closed
mtxr opened this issue Sep 23, 2018 · 9 comments
Closed

Formatting comments, new lines are not kept #50

mtxr opened this issue Sep 23, 2018 · 9 comments
Labels

Comments

@mtxr
Copy link
Contributor

mtxr commented Sep 23, 2018

Here: mtxr/vscode-sqltools#97

Issue Type

  • Bug
  • Enhancement
  • Feature Request
  • Question
  • Other

Prerequisites (For bugfixes)

  • Are you running the latest version?
  • Did you check the logs?
  • Did you check the Setup?

Description

I've got a similar bug to this issue (#87) when formatting simple queries with comments :

SELECT * FROM user;

-- RESET ALL PASSWORD REQUESTS

UPDATE user SET password_requested_at = NULL, confirmation_token = NULL;

Steps to Reproduce (For bugfixes)

  1. Create a .sql file with file content type set as SQL
  2. Put those simple queries into the file
  3. Apply format document command

Expected behavior: [What you expected to happen]

The file should be formatted as bellow, the comment should keep its new lines (before and after)

SELECT
    *
FROM
    user;

-- RESET ALL PASSWORD REQUESTS

UPDATE
    user
SET
    password_requested_at = NULL,
    confirmation_token = NULL;

Actual behavior: [What actually happened]

The comment is after the last sql part without any new lines.

SELECT
    *
FROM
    user;-- RESET ALL PASSWORD REQUESTS
UPDATE
    user
SET
    password_requested_at = NULL,
    confirmation_token = NULL;

Versions

  • Version: v0.15.0 (vscode 1.26.1)
  • OS: fedora 28
  • SGDB: PostgresSQL (but it doesn't look like to be related to a specific database vendor)
@jylertones
Copy link
Contributor

I'm also noticing one more case that ends up eating my statement terminator semicolon:

Query:

SELECT * FROM user
-- no where clause here
;

Expected:

SELECT
  *
FROM
  user 
-- no where clause here
;

Actual:

SELECT
  *
FROM
  user -- no where clause here;

@mtxr I'm happy to help fix, have you worked on this at all?

@jylertones
Copy link
Contributor

Digging into the code on this, I do not think that my issue is related to this.

My issue happens because sql-formatter removes whitespace, including newlines, when adding the semicolon at the end of the line.

For what it's worth, I don't see evidence to support that sql-formatter while preserve multiple line breaks between any token that it parses. Your desired output may require sql-formatter to be more flexible with preserving existing whitespace in some cases.

@nene
Copy link
Collaborator

nene commented Oct 28, 2018

Formatting of comments is a tricky thing in general.

At the moment sql-formatter tries to place all line-comments at the end of the preceding code. An alternative would be to always put the comment on a line of its own, but of course that would then break those comments that were meant to be on the same line as code.

I think to properly solve it, we'll need to distinguish at parse time between comments between code and on the same line as code, and then format them accordingly in output. Then you could get the following:

SELECT
    *
FROM
    user;
    -- RESET ALL PASSWORD REQUESTS
UPDATE
    user
SET
    password_requested_at = NULL,
    confirmation_token = NULL;

Another problem (exemplified above) is that sql-formatter doesn't really understand multiple SQL queries. Originally it was built to solve the task of formatting just a single query. We'll need to teach it, that ; ends a query. Then we could separate queries with empty line (not 100% sure if that would be good - so fare nobody has complained about queries not being separated by empty line) and get something like this:

SELECT
    *
FROM
    user;

-- RESET ALL PASSWORD REQUESTS
UPDATE
    user
SET
    password_requested_at = NULL,
    confirmation_token = NULL;

So yeah. I'd consider it definitely a bug. But it requires quite a bit of work.

@nene
Copy link
Collaborator

nene commented Oct 28, 2018

The simpler approach is to first implement the understanding of multiple queries. Perhaps it can be as simple as just whenever we see ; - it's the end of the query.

That could solve the concrete issue brought up in this ticket. Though issues with comments inside queries would still remain. But I'd suspect comments between queries to be more common use case.

@osv
Copy link
Contributor

osv commented Dec 28, 2018

Tested on 2.3.2:

-- This is the first statement
select * from foo where color = "purple";

-- This is the second statement
select * from foo join bar on foo.id = bar.foo_id where bar.count > 100;
-- This is the first statement
select
  *
from
  foo
where
  color = "purple";-- This is the second statement
select
  *
from
  foo
  join bar on foo.id = bar.foo_id
where
  bar.count > 100;

@mtxr
Copy link
Contributor Author

mtxr commented Feb 4, 2019

@zeroturnaround any updates on this?

@ecthomps
Copy link

ecthomps commented Jun 19, 2019

select col1, col2
-- comment 1
from
tbl1
left join tbl2 ON tb1.col1 = tbl2.col1
-- comment 2
left join tbl3 ON tbl1.col2 = tbl3.col3

expected output
select
-- comment 1
col1,
col2
from
tb1
left join tbl2 ON
tb1.col1 = tbl2.col1
-- comment 2
left join tbl3 ON
tbl1.col2 = tbl3.col3

actual output
select
-space- comment 1
col1,
col2
from
tb1
left join tbl2 ON
tb1.col1 = tbl2.col1
-space- comment 2
left join tbl3 ON
tbl1.col2 = tbl3.col3

there's a space btn the dash lines in my output, making the comments to be invalid sql comments

@carcinocron
Copy link

carcinocron commented Aug 11, 2019

Would love more than one newline between statements in general.

@nene
Copy link
Collaborator

nene commented Jan 27, 2021

This should be fixed now with the 3.1.0 release.

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

6 participants