Skip to content

[#41] Delete codan markers #108

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

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

ghentschke
Copy link
Contributor

prior to open a file with the LSP based C/C++ editor. When a C/C++ file has been opened prior by the old CDT C/C++ editor the codan markers are persistent and must be removed from the resource.

fixed #41

prior to open a file with the LSP based C/C++ editor. When a C/C++ file
has been opened prior by the old CDT C/C++ editor the codan markers are
persistent and must be removed from the resource.

fixed eclipse-cdt#41
Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

I think this is an interesting and simple approach. I guess the longer term solution will be to having some preference / setting that disables codan (and CDT parser) when lsp is in use, and when that setting is enabled all the codan errors are removed.

@ghentschke ghentschke merged commit 164d6f6 into eclipse-cdt:master Jun 6, 2023
@ghentschke
Copy link
Contributor Author

@jonahgraham Thank you!

@ruspl-afed
Copy link
Member

I think this is an interesting and simple approach

Yes, if it will delete markers inside WorkspaceJob and not in UI thread.

@jonahgraham
Copy link
Member

@ghentschke, I think @ruspl-afed makes a good point, it may be better to do the work asynchronously. I'm not sure if needs a full workspace job as that locks whole workspace, deleteMarkers will only lock on the resource being modified.

@ghentschke
Copy link
Contributor Author

@ruspl-afed and @jonahgraham Your are right. I'll provide a PR for that.

@ruspl-afed
Copy link
Member

I'm not sure if needs a full workspace job as that locks whole workspace

We don't need to lock the whole workspace, locking corresponding IResource looks enough here

@ruspl-afed
Copy link
Member

And this is the second time when we do modifications from "isEnabled" method, we need to improve this as well

@ghentschke ghentschke deleted the remove-marker branch June 7, 2023 05:01
@jonahgraham jonahgraham added this to the 1.0.0 milestone Sep 18, 2023
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.

Remove markers from previous opening
3 participants