Add support for CodeChecker parse - #37

Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged

Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom
Discookie:ericsson-cc-parse

Conversation

@Discookie

@DiscookieDiscookie commented Dec 20, 2021•
edited by csordasmarton
Loading

Copy link
Copy Markdown
Collaborator

Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.

Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.
Currently the process priorities are version > parse > analyze > anything else.
A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.

Refactored Diagnostics to use the new format provided by CodeChecker parse.
This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.
Otherwise, the feature-set is on par with the old Diagnostic system.

Added a minimum CodeChecker version check for all executions, without a user alert for now.
Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.
A user alert on an old CC will be added soon to this PR.

The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.

csordasmarton
csordasmarton suggested changes Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts Outdated
try {
// Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
const outputLines = processOutput.split('\n');
const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;

@csordasmartoncsordasmartonDec 20, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).

By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version

@DiscookieDiscookieDec 22, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.

Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts Outdated
Comment threadsrc/backend/executor/bridge.ts
const minimum = [6, 18, 1];

if (version < minimum) {
this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);

@csordasmartoncsordasmartonDec 20, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?

@DiscookieDiscookieDec 22, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.

Not sure what links to include there, but for now they're good enough.

Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
return undefined;
}

public getAnalyzeCmdLine(...files: Uri[]): string | undefined {

@csordasmartoncsordasmartonDec 20, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.

From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.

@DiscookieDiscookieDec 22, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.

Comment threadsrc/backend/executor/bridge.ts
].join(' ');
}

public getParseCmdLine(...files: Uri[]): string | undefined {

@csordasmartoncsordasmartonDec 20, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:

>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0

Are we running the parse command multiple times? Also why are these differents?

@DiscookieDiscookieDec 21, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):

CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60 in 2d14270

window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);

I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.
Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.

@csordasmartoncsordasmartonDec 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.

Comment threadsrc/sidebar/views/reports.ts Outdated
Comment threadsrc/editor/executor.ts Outdated
Comment threadsrc/backend/executor/bridge.ts
@Discookie

Discookie commented Dec 22, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.

@DiscookieDiscookie linked an issue Dec 22, 2021 that may be closed by this pull request
Verify if CodeChecker is available #29
Closed
csordasmarton
csordasmarton suggested changes Dec 23, 2021
View reviewed changes

@csordasmartoncsordasmarton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!

Comment threadsrc/backend/executor/bridge.ts
].join(' ');
}

public getParseCmdLine(...files: Uri[]): string | undefined {

@csordasmartoncsordasmartonDec 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.

Comment threadsrc/backend/executor/bridge.ts Outdated
Comment on lines +344 to +349
`CodeChecker: Version ${minimum.join('.')} not supported. ` +
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',

@csordasmartoncsordasmartonDec 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change
`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',

And also add an extra button to open the Settings of this extension.

@DiscookieDiscookieDec 23, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.

Comment threadsrc/backend/executor/bridge.ts Outdated
csordasmarton
csordasmarton approved these changes Dec 23, 2021
View reviewed changes

@csordasmartoncsordasmarton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@csordasmarton
csordasmarton merged commit 455acc7 into Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton mentioned this pull request Jan 3, 2022
Closed
@csordasmartoncsordasmarton added this to the 0.0.1 milestone Jan 24, 2022
This was referenced Jan 26, 2022
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

refactoring

Projects

None yet

Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command Verify if CodeChecker is available Skip suppressed reports

2 participants

@Discookie@csordasmarton
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Add support for CodeChecker parse - #37
Merged
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Dec 23, 2021
Merged
Add support for CodeChecker parse#37
csordasmarton merged 5 commits into
Ericsson:mainfrom 
Discookie:ericsson-cc-parse
Conversation
@Discookie
@DiscookieDiscookie
commented
Dec 20, 2021•

edited by csordasmarton

Loading

Copy link


Copy Markdown

Collaborator
Adds support for the CodeChecker parse command, new with CC version 6.18.0.
Fixes#31, fixes#20, fixes#29.
Refactored the executor infrastructure, which allows for running and prioritizing multiple processes.

Currently the process priorities are version > parse > analyze > anything else.

A queue system is in place to be able to queue, clear and replace multiple types of tasks at the same time.
Refactored Diagnostics to use the new format provided by CodeChecker parse.

This means losing some information, such as the depth of the repr. steps stack. Replacement for that will be in another PR.

Otherwise, the feature-set is on par with the old Diagnostic system.
Added a minimum CodeChecker version check for all executions, without a user alert for now.

Currently the minimum version is 6.18.1, for the --file flag of CodeChecker parse.

A user alert on an old CC will be added soon to this PR.
The new Executor infrastructure could really use some tests, but for now it is untested. Expect tests to land in another PR.
csordasmarton
csordasmarton
suggested changes
Dec 21, 2021
View reviewed changes
Comment threadsrc/backend/executor/bridge.ts
Outdated

 try {
 // Structure: CodeChecker analyzer version: \n {"Base package version": "M.m.p", ...}
 const outputLines = processOutput.split('\n');
 const dataLine = outputLines.findIndex((line) => line.search(/CodeChecker analyze version:/))+1;
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

CodeChecker version command is not a valid JSON format. I already create a patch to solve this problem (Ericsson/codechecker#3558).
By the way we are using only the analyzer part of the CodeChecker so we can get the analyzer version with the following command: CodeChecker analyzer-version
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Thanks for the analyzer-version! The version check now uses that one. Removed the rest of the trimming code.
Comment threadsrc/backend/types/parse_result.ts
Comment threadsrc/backend/executor/bridge.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
 const minimum = [6, 18, 1];

 if (version < minimum) {
 this._bridgeMessages.fire(`>>> Unsupported CodeChecker version ${version}\n`);
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I had a 6.17.0 CodeChecker version in my PATH but I don't see any notification about this. The analysis was successfully finished but I don't see any reports in the CodeChecker Overview tab. So I checked the debug logs and I saw this message there. A tipical user will not find this information, so can we show a popup with this information in this case?
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Added notifications for both unsupported and missing cases.
Not sure what links to include there, but for now they're good enough.
Comment threadsrc/backend/executor/bridge.ts
Comment threadsrc/backend/executor/bridge.ts
 return undefined;
 }

 public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's assume that I have two files a.cpp and b.cpp and I open these files (I don't modify any of them). If I open a.cpp the GUI says analysis in progress / finished. If I open b.cpp it will show the same message in the footer. If I select a.cpp again it will still show me the same.
From the logs I think we do not really run the analysis but the messages are missleading. So I recommend not to show these messages when we do not relly running the analysis.
@DiscookieDiscookieDec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed for now, so that parse / version doesn't update the bottom bar. Same condition as showing / hiding the progress bar.
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 20, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If I open a source file in the logs I will see the following messages:
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1"
>>> Process 'CodeChecker parse' exited with code 0
>>> Starting process 'CodeChecker parse'> /CodeChecker/bin/CodeChecker parse /home/username/helloworld/.codechecker -e json --file "/extension-output-codechecker.codechecker-#1""/home/username/helloworld/multiple/a.cpp"
>>> Process 'CodeChecker parse' exited with code 0
Are we running the parse command multiple times? Also why are these differents?
@DiscookieDiscookieDec 21, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

We run the parse command on all 'visible text editors changed' events (opens, tab switches, etc.):
CodecheckerVSCodePlugin/src/backend/processor/diagnostics.ts

Line 60
in
2d14270
window.onDidChangeVisibleTextEditors(this.onDocumentsChanged,this,ctx.subscriptions);
I'm not sure how it handles opening new windows, since it's a VS Code builtin, but it hasn't changed from before.

Will need to investigate further about whether this is an issue or not, but I don't believe a fix will make it into this PR.
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/sidebar/views/reports.ts
Outdated

Comment threadsrc/editor/executor.ts
Outdated

Comment threadsrc/backend/executor/bridge.ts
@Discookie
Discookie
commented
Dec 22, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Added notifications for missing and outdated CodeChecker installs. Fixes #29 as well.
@DiscookieDiscookie
linked an issue
Dec 22, 2021
that may be
closed
by this pull request


Verify if CodeChecker is available
#29

Closed

csordasmarton
csordasmarton
suggested changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I have just some tiny comments otherwise LGTM!
Comment threadsrc/backend/executor/bridge.ts
 ].join(' ');
 }

 public getParseCmdLine(...files: Uri[]): string | undefined {
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Okay, I created a separate issue for this problem: #39.
Comment threadsrc/backend/executor/bridge.ts
Outdated


Comment on lines

+344
 to 
+349

 `CodeChecker: Version ${minimum.join('.')} not supported. ` +
 'Update CodeChecker, or check the extension settings.',
 'Open releases',
 'Installation guide',
@csordasmartoncsordasmartonDec 23, 2021

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

The message is not correct, because you say that the minimum supported version is not supported. I recommend to refactor this message to something like this:

Suggested change

`CodeChecker: Version ${minimum.join('.')} not supported. `+
'Update CodeChecker, or check the extension settings.',
'Open releases',
'Installation guide',
`CodeChecker version you are using (${version.join('.')}) is not supported. `+
`The minimum supported version is ${minimum.join('.')}. Please update to the `+
`latest CodeChecker version, or check the extension settings.`,
'Open releases',
'Installation guide',
'Settings',
And also add an extra button to open the Settings of this extension.
@DiscookieDiscookieDec 23, 2021

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Fixed as requested, and added the Open settings button to the "Not found" notif as well.
Comment threadsrc/backend/executor/bridge.ts
Outdated

csordasmarton
csordasmarton
approved these changes
Dec 23, 2021
View reviewed changes
@csordasmartoncsordasmarton
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM!
@csordasmarton
csordasmarton
merged commit 455acc7
into
Ericsson:mainDec 23, 2021
@csordasmartoncsordasmarton
mentioned this pull request
Jan 3, 2022

Closed

@csordasmartoncsordasmarton
added this to the 0.0.1 milestone
Jan 24, 2022

This was referenced Jan 26, 2022

Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@csordasmartoncsordasmartoncsordasmarton approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

refactoring

Projects


None yet


Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

Use json output of CodeChecker parse command

Verify if CodeChecker is available

Skip suppressed reports


2 participants

@Discookie@csordasmarton