Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gui] Fix exporting checker statistics to CSV #3362

Merged

Conversation

csordasmarton
Copy link
Contributor

It is possible that a checker name contains a hashmark character
(clang-diagnostic-#warnings, clang-diagnostic-#pragma-messages).
Hashmark is a valid URL character but it starts the hash fragment and
for this reason it needs to be escaped when we export the statistics
to CSV.

.join("")
// Hashmark (#) is a valid URL character but it starts the hash
// fragment and for this reason it needs to be escaped.
.replace("#", encodeURIComponent("#"));
Copy link
Contributor

Choose a reason for hiding this comment

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

The pattern in this function call is a "string" instead of a "regex", so the first occurrence will be replaced only.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace
Other note: encodeURIComponent("#") could be stored in a variable which provides that only one call will be done of this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. I change it to replaceAll so it will replace every occurances of hashmarks.

This is not frequently used function and encoding hashmark is not a slow operation, so I don't think it worth to move it into a separate variable.

It is possible that a checker name contains a hashmark character
(`clang-diagnostic-#warnings`, `clang-diagnostic-#pragma-messages`).
Hashmark is a valid URL character but it starts the hash fragment and
for this reason it needs to be escaped when we export the statistics
to CSV.
@csordasmarton csordasmarton force-pushed the fix_checker_statistics_export branch from 9fd5f78 to cc0aaab Compare July 8, 2021 08:41
@csordasmarton csordasmarton requested a review from zomen2 July 8, 2021 08:44
@csordasmarton csordasmarton merged commit 33021ca into Ericsson:master Jul 8, 2021
@csordasmarton csordasmarton deleted the fix_checker_statistics_export branch January 28, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants