Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 1, 2026

  • Modify didClose in BSLTextDocumentService to check clientSupportsPullDiagnostics before publishing empty diagnostics
  • Add test coverage for the conditional behavior in BSLTextDocumentServiceTest:
    • didClosePublishesEmptyDiagnosticsWhenClientDoesNotSupportPullDiagnostics() - verifies publishEmptyDiagnosticList is called when client doesn't support pull diagnostics
    • didCloseDoesNotPublishEmptyDiagnosticsWhenClientSupportsPullDiagnostics() - verifies publishEmptyDiagnosticList is NOT called when client supports pull diagnostics
  • Simplified test approach by mocking clientCapabilitiesHolder directly instead of using event publishing
  • Run tests to verify all changes work correctly
  • Run CodeQL security check
Original prompt

Проверить, надо ли слать пустой publishDiagnostics при didClose, если клиент поддерживает textDocument/diagnostics


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI changed the title [WIP] Check empty publishDiagnostics on didClose event Skip empty publishDiagnostics on didClose when client supports pull diagnostics Jan 1, 2026
Copilot AI requested a review from nixel2007 January 1, 2026 20:14
@nixel2007 nixel2007 marked this pull request as ready for review January 1, 2026 21:37
Copilot AI review requested due to automatic review settings January 1, 2026 21:37
@nixel2007 nixel2007 enabled auto-merge January 1, 2026 21:37
@github-actions
Copy link
Contributor

github-actions bot commented Jan 1, 2026

Test Results

  312 files   - 2 496    312 suites   - 2 496   3m 4s ⏱️ - 50m 31s
1 151 tests +    2  1 151 ✅ +    2  0 💤 ±0  0 ❌ ±0 
1 151 runs   - 9 190  1 151 ✅  - 9 190  0 💤 ±0  0 ❌ ±0 

Results for commit e0fe3a4. ± Comparison against base commit e5b5781.

This pull request removes 8 and adds 6 tests. Note that renamed tests count towards both.
#, count=1
, count=0
, count=1
A, count=1
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [2] content=
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [6] content=
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [7] content=#
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [8] content=
com.github._1c_syntax.bsl.languageserver.BSLTextDocumentServiceTest ‑ didCloseDoesNotPublishEmptyDiagnosticsWhenClientSupportsPullDiagnostics()
com.github._1c_syntax.bsl.languageserver.BSLTextDocumentServiceTest ‑ didClosePublishesEmptyDiagnosticsWhenClientDoesNotSupportPullDiagnostics()
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [2] content=
, count=0
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [6] content=
A, count=1
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [7] content=#
, count=1
com.github._1c_syntax.bsl.languageserver.diagnostics.MetadataObjectNameLengthDiagnosticTest ‑ [8] content=
#, count=1

♻️ This comment has been updated with latest results.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements LSP spec-compliant behavior by preventing servers from sending publishDiagnostics notifications when clients use the pull diagnostic model (textDocument/diagnostic). The change ensures that empty diagnostics are not published on document close when the client supports pull diagnostics.

Key Changes

  • Added conditional check in didClose method to skip publishing empty diagnostics when clientSupportsPullDiagnostics is true
  • Follows the same pattern already established in the validate method

@nixel2007
Copy link
Member

@copilot что со сборкой?

@nixel2007
Copy link
Member

@copilot Завис?

auto-merge was automatically disabled January 2, 2026 00:00

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Jan 2, 2026

@copilot Завис?

Added test coverage for the didClose diagnostic publishing behavior as suggested by the code review. Commit 0ba2aac adds two tests that verify:

  • publishEmptyDiagnosticList is called when client doesn't support pull diagnostics
  • publishEmptyDiagnosticList is NOT called when client supports pull diagnostics

Copilot AI requested a review from nixel2007 January 2, 2026 08:45
@nixel2007 nixel2007 enabled auto-merge January 2, 2026 08:47
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 2, 2026

@nixel2007 nixel2007 merged commit a11189a into develop Jan 2, 2026
40 of 41 checks passed
@nixel2007 nixel2007 deleted the copilot/check-empty-publish-diagnostics branch January 2, 2026 09:42
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