Conversation
- create fUserMessages, fCodeToolErrors, fParserErrors to hold diagnostic param items - Create infrastructure methods to add/clear into theese fields
- Only add CodeTool error if there are no error previously on that location - Improve CodeToolCheckSyntax to scan unknown identifier
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To be effective, this PR should compiled using FPC main branch (as time of writing, i use 6460b5bae7) with Lazarus also on main branch. Compile using stable FPC and Lazarus give no notable differences to our current Publish Diagnostic handling state.
That is because fcl-passrc still being actively developed and "multi error" handling eventhough has been merged to main branch, it still not merged to any "stable" branch. I think even FPC's latest
release_3_2_4_rc2tag doesn't have theese commits merged. @mvancanneyt maybe able to confirm this.Now let's go into what this PR do.
As I mentioned in #163 there are two paths that triggers
textDocument/publishDiagnosticthat is viaCodeToolBoss, used byserverprotocol/PasLS.*.pasand viaTSourceParserwhich is used by PasLS.Synchronization.pas.The issue with current implementation is,
TPublishDiagnosticsare created on the fly before sending Publish Diagnostic notification. This would make previous errors, for example that has been sent byStrictSyntaxCheck()cleared when new error from Goto Definition command issued. Any new notification also clear errors from another open files.I propose to use persistent
TPublishDiagnosticswhich owned and managed byTDiagnosticHandler. This object instance have 3 field:fUserMessages,fCodeToolErrors, andfParserErrors.fCodeToolErrors, andfParserErrorsare mapped to a URI, so every file can have their ownTDiagnosticItems. Corresponding fields are cleared then re-populated each CheckSyntax() callI also modify CodeToolsCheckSyntax() to gather all identifier in file then check if that identifier is defined. This will trigger CodeToolBoss' errors and add those errors into
TDiagnosticItemsmapped to current file. Unfortunately CodeToolBoss still report many Pascal keyword likepublic,private,protectedas "identifier not found". I feel that's downside compile against "non-stable" branch of compiler. But I think it still valuable progress toward publishDiagnostic handling correctness.