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

Unsupported tags dl and dt #5414

Open
vnaskos-sonar opened this issue May 8, 2024 · 6 comments
Open

Unsupported tags dl and dt #5414

vnaskos-sonar opened this issue May 8, 2024 · 6 comments

Comments

@vnaskos-sonar
Copy link
Contributor

After the #5406 on SLVS it was identified that there is an issue with the following rule due to unsupported tags dl and dt

S6966

<p>Using a synchronous method instead of its <a href="https://learn.microsoft.com/en-us/dotnet/csharp/asynchronous-programming/">asynchronous</a>
counterpart in an <code>async</code> method blocks the execution and is considered bad practice for several reasons:</p>
<dl>
  <dt>
@vnaskos-sonar vnaskos-sonar changed the title Unsupported tags dl and dt Unsupported tags dl and dt May 8, 2024
@martin-strecker-sonarsource

Rendering is supported by other consumer such as SonarQube:
https://peach.aws-prd.sonarsource.com/coding_rules?q=S6966&open=csharpsquid%3AS6966

image

@georgii-borovinskikh-sonarsource
Copy link
Contributor

Hey @martin-strecker-sonarsource
The issue is that we can't directly display HTML in the IDE. We convert it to XAML, and we need to update the conversion code for each new html tag that is used. It's technically not an issue on your side, it's just a bit of a painful process for us to update the converter for tags that are used only in one rule. If you think that this tag is really needed, then we will have to fix it on our side at some point. But in general, it is easier to fix it on your side than on ours.

@martin-strecker-sonarsource

We can change the rule rspec but we also do have a second rule in the pipeline with definition lists:

https://github.com/SonarSource/rspec/pull/3668/files#diff-0429ce0e4c469b55eba4b8851ed523317e5c786281f08520a665b956f4c2541a

If we change the rules, the rpsec CI needs to be adopted to flag definition lists as unsupported. Doing so might be as much of an effort as to support it on your side. What do you think?

@georgii-borovinskikh-sonarsource
Copy link
Contributor

@martin-strecker-sonarsource if it's multiple rules that reuse this feature, then it definitely makes more sense to add the support on our side

@pavel-mikula-sonarsource
Copy link
Contributor

There exists a showcase plugin that contains show-case RSPEC that demonstrates all legal formating that rules can use, and that products must support.

Actual rules can be found here.

I have not seen <dl> and <dt> being used there, therefore rules should not expect products to be able to display those.

We need to fix the RSPEC to conform to the existing set of supported HTML nodes.

If there's a real need, we can talk with CFamily Squad about what it takes to add new nodes, reach an agreement with products and update the showcase. Only after that we can start using new nodes.

@pavel-mikula-sonarsource
Copy link
Contributor

Just FYI - it doesn't change anything about this ticket: I've created #5422 to create UT based on the showcase plugin, so we learn about new formatting elements before the problem happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants