Preserve comment lines and leading commas in SQL formatting

[satt-hri]

Hi,

I noticed that when formatting SQL with inline comments and leading commas, the formatter moves comments to the end of the previous line and changes the comma placement.

Example SQL I want to keep:

INSERT INTO sco_progress_log(
rec_datetime
,log_type

/* __CF_IF5__ */
,caller_sco_id
/* __CF_IF6__ */

,complete_date
)
SELECT
__CF_PARAM0__
,'rireki'
,NULL

/* __CF_IF7__ */
,__CF_PARAM1__
/* __CF_IF8__ */

FROM sco_progress ins_data
WHERE user_id = __CF_PARAM2__
AND course_id = __CF_PARAM3__
AND sco_id = __CF_PARAM4__

Formatted result:

INSERT INTO
	sco_progress_log (
		rec_datetime,
		log_type
		/* __CF_IF5__ */
,
		caller_sco_id
		/* __CF_IF6__ */
,
		complete_date
	)
SELECT
	__CF_PARAM0__,
	'rireki',
	NULL
	/* __CF_IF7__ */
,
	__CF_PARAM1__
	/* __CF_IF8__ */

FROM
	sco_progress ins_data
WHERE
	user_id = __CF_PARAM2__
	AND course_id = __CF_PARAM3__
	AND sco_id = __CF_PARAM4__

Issues:

1. Comments (/* __CF_IFx__ */) are moved to the previous line instead of staying on their own line.
2. Leading commas are reformatted.

Is there a way to preserve comment lines and leading commas? If not, could this be considered as a feature request?

Thanks!

[karlhorky]

Oh interesting, have you tried using -- comments instead?

INSERT INTO sco_progress_log(
rec_datetime
,log_type

-- __CF_IF5__
,caller_sco_id
-- __CF_IF6__

,complete_date
)
SELECT
__CF_PARAM0__
,'rireki'
,NULL

-- __CF_IF7__
,__CF_PARAM1__
-- __CF_IF8__

FROM sco_progress ins_data
WHERE user_id = __CF_PARAM2__
AND course_id = __CF_PARAM3__
AND sco_id = __CF_PARAM4__

I'm not sure it will result in exactly what you're aiming for, but maybe it's closer?

[satt-hri]

Thank you for your reply.
I have tried using -- for comments, but it did not work.
Perhaps both -- and /* */ require testing and debugging.

[karlhorky]

I have tried using -- for comments, but it did not work.

Can you show the output from -- and write why it is incorrect?

I have not had problems with formatting of -- comments in our medium-size codebases.

[satt-hri]

this is my sql .

INSERT INTO sco_progress_log(
sco_type
,sco_type_name
,content_type_id
,content_type_name
-- __CF_IF6__ 
,caller_sco_id
-- __CF_IF7__ 
,completion_status
)SELECT
__CF_PARAM0__
,'rireki'
,NULL
,__CF_PARAM1__
-- __CF_IF8__ 
,__CF_PARAM2__
-- __CF_IF9__ 
,ins_data.completion_status

FROM sco_progress ins_data
WHERE		user_id		= __CF_PARAM3__
AND		course_id	= __CF_PARAM4__
AND		sco_id		= __CF_PARAM5__ 

Formatted result:

INSERT INTO
	sco_progress_log (
		sco_type,
		sco_type_name,
		content_type_id,
		content_type_name
		-- __CF_IF6__ 
,
		caller_sco_id
		-- __CF_IF7__ 
,
		completion_status
	)
SELECT
	__CF_PARAM0__,
	'rireki',
	NULL,
	__CF_PARAM1__
	-- __CF_IF8__ 
,
	__CF_PARAM2__
	-- __CF_IF9__ 
,
	ins_data.completion_status
FROM
	sco_progress ins_data
WHERE
	user_id = __CF_PARAM3__
	AND course_id = __CF_PARAM4__
	AND sco_id = __CF_PARAM5__ 

---

const formatOption: FormatOptionsWithLanguage = {
	language: "sql",
	useTabs: true,
	tabWidth: 1,
	keywordCase: "upper",
	expressionWidth: 30,
};

[karlhorky]

Ok, right. So it seems like it is more related to the leading commas.

If I switch to trailing commas, the output is improved:

Input:

INSERT INTO sco_progress_log(
sco_type
,sco_type_name
,content_type_id
,content_type_name,
-- __CF_IF6__ 
caller_sco_id,
-- __CF_IF7__ 
completion_status
)SELECT
__CF_PARAM0__
,'rireki'
,NULL
,__CF_PARAM1__,
-- __CF_IF8__ 
__CF_PARAM2__,
-- __CF_IF9__ 
ins_data.completion_status

FROM sco_progress ins_data
WHERE		user_id		= __CF_PARAM3__
AND		course_id	= __CF_PARAM4__
AND		sco_id		= __CF_PARAM5__ 

Output:

INSERT INTO
  sco_progress_log (
    sco_type,
    sco_type_name,
    content_type_id,
    content_type_name,
    -- __CF_IF6__ 
    caller_sco_id,
    -- __CF_IF7__ 
    completion_status
  )
SELECT
  __CF_PARAM0__,
  'rireki',
  NULL,
  __CF_PARAM1__,
  -- __CF_IF8__ 
  __CF_PARAM2__,
  -- __CF_IF9__ 
  ins_data.completion_status
FROM
  sco_progress ins_data
WHERE
  user_id = __CF_PARAM3__
  AND course_id = __CF_PARAM4__
  AND sco_id = __CF_PARAM5__

SQL Formatter had an option previously for comma formatting, but it was removed and is currently not planned for re-implementation:

I currently don't have any plans to re-implement this feature. I'm open to pull requests if anybody is interested in creating this feature properly.

[satt-hri]

Wow, it worked! Thank you!
I mean, it really formats nicely—thanks for your help.
I also want to ask: in SQL, is it not allowed to put the comma before the item? Is there any reason for that?

INSERT INTO
	sco_progress_log (
		sco_type,
		sco_type_name,
		content_type_id,
		content_type_name,
		-- __CF_IF6__ 
		caller_sco_id,
		-- __CF_IF7__ 
		completion_status
	)
SELECT
	__CF_PARAM0__,
	'rireki',
	NULL,
	__CF_PARAM1__,
	-- __CF_IF8__ 
	__CF_PARAM2__,
	-- __CF_IF9__ 
	ins_data.completion_status
FROM
	sco_progress ins_data
WHERE
	user_id = __CF_PARAM3__
	AND course_id = __CF_PARAM4__
	AND sco_id = __CF_PARAM5__ 

[nene]

Thanks for reporting.

The problem is that the formatter only modifies the whitespace between tokens and avoids changing the order of tokens.

So this SQL:

SELECT
  foo
  /* comment */
  ,bar

will get parsed to list of tokens like this:

<keyword SELECT>
<identifier foo>
<comment /* comment */>
<comma>
<identifier bar>

The formatter would like to place the comma after foo, but there's a comment in-between. It does not change the order of tokens, so there's not much it can do.

Possibly some pre-processing could be done on the token stream to ensure that commas are never after comments. I don't think there's ever a scenario where that would change the meaning of the code. At least I can't think of any.

The bottom line here is that the formatter doesn't support the comma-first style. It used to support it in the past, but the implementation of it was bug-ridden and was removed. Similarly the plan is to drop support for the different indentation styles. That's because it's hard to support even one indentation style well. Attempting to support multiple is just too much work.

[satt-hri]

Thank you for your reply.

[karlhorky]

@satt-hri does this resolve the problem for you? Do you think it's ok to close this issue now?