[mangomac]

Why

Requested in beekeeper-studio/beekeeper-studio#4874 (comment) . SQL Server stores the supplied module definition, so dropping a procedure's header comments loses part of that definition. The caller should not need a second comment parser.

Change

Add includeLeadingComments, default off. With it enabled, text and start include the leading comment prefix and adjacent whitespace. end, classification and parameter extraction stay unchanged. Empty semicolon delimiters are not included. Whitespace without a comment does not move the start.

The implementation walks the existing tokens, not a second raw-text regular expression, so comment markers inside strings remain strings. It applies to all dialects only when a caller opts in.

Verification

• Six regression/default tests, three fail before the implementation and all pass after.
• npm test: 829 passing.
• npm run build and npm run lint: pass.
• All four remote file blobs match the tested local files.

This is the library half of Beekeeper Studio #4874. Once an accepted library release is available, that PR can use the option instead of its local helper. No package version or release workflow is changed here. I have not verified storage in a live SQL Server.

[azmy60]

@mangomac question: do we just need to run mssql script without stripping out any comments in particular, or is keeping the leading comments and stripping out the rest a common practice for mssql?

[mangomac]

The goal is to preserve comments in the submitted module definition, not to keep only the header and strip the body. I am not proposing that as an MSSQL convention.

The library already returns a substring of the original SQL between each statement's start and end; it does not remove comments inside those bounds. I just checked this with an MSSQL CREATE PROCEDURE containing both a -- body line and /* body block */: both remain in text with the option off or on. The only difference is that the option also includes -- header before CREATE.

This option does not promise to retain the entire script byte-for-byte. A trailing comment after a terminating semicolon is outside that statement's existing bounds; end is unchanged here. The issue behind Beekeeper #4874 was specifically the missing header before the module definition. This opt-in extends that boundary without changing existing consumers' results. If you prefer a broader whole-script/comment-ownership contract, that needs separate semantics rather than silently changing text for every caller.

[azmy60]

This is a bit misplaced. Please move this to ##API (line 147) as a third parameter.

[azmy60]

Are we sure about including leading whitespace? Just want to make sure if this a correct behavior in general, because the option says "leading comments".

[mangomac]

Moved the option into the API options list as the third entry, with its example under API, in 3907300. README on the remote branch matches the checked local file. Re-ran the suite (829 passing), build and lint.

On whitespace: the current implementation includes the contiguous comment/whitespace prefix, so -- header\nCREATE ... starts at the two spaces. Whitespace between a header comment and the keyword must remain in that substring, but whitespace before the first comment is a separate choice. A whitespace-only prefix still starts at the keyword, and empty semicolon delimiters stay excluded; the tests pin both cases.

Beekeeper's need is to retain the header comments before a module definition. That does not require preserving whitespace before the first comment. I chose the contiguous prefix to avoid trimming that source slice, not because SQL Server requires the indentation. Starting at the first comment would also meet that consumer's need and fit the option name more narrowly. Would you prefer that boundary? I have left the behavior unchanged while clarifying rather than treating the question as a request to change it.

[azmy60]

@mangomac I think we should separate comments and whitespace. We should have includeLeadingComments and includeLeadingWhitespace. This makes the API sound more consistent.

[mangomac]

Implemented the separate options in 903de89. Both default to false, preserving existing callers' keyword-based offsets and text.

For -- header\n SELECT 1;:

• Neither: starts at SELECT.
• includeLeadingComments only: starts at -- header.
• includeLeadingWhitespace only: starts at the two spaces before SELECT, without the comment.
• Both: starts at the two spaces before -- header.

The existing tokenizer owns a line comment's terminating newline as part of that comment token, so whitespace-only mode excludes that newline too. Whitespace between an included header and the keyword stays in the contiguous source substring. Semicolon delimiters remain excluded; end, classification and parameters stay unchanged.

Updated the API docs and tests for all four combinations, whitespace-only prefixes, comment/delimiter barriers, separate statement headers and parameter compatibility. All 839 tests, build and lint pass locally. Against the previous implementation, the revised boundary tests expose seven failures. All four remote files match the tested files. No CI checks have been reported on this head.

This supersedes the original PR description's single-option/adjacent-whitespace behavior. Beekeeper can opt into both options once the library change is accepted and released; its PR remains unchanged until then. No release or package publishing is included here.

[azmy60]

LGTM

[mangomac]

Following up on the approved change: is there anything else needed before this can merge and be included in a release? Beekeeper's dependent fix (#4874) is waiting for a released version containing these options. Both options remain off by default, and I haven't published or changed any package version.