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

Escaping name and desc properties before converting to HMTL #253

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

krcb197
Copy link
Contributor

@krcb197 krcb197 commented Feb 24, 2025

This PR addresses some undesirable things that can happen with the legal system RDL format string but would result in undesirable problems in HTML. See the following issue tickets:

There are two parts to this:

  1. The systemRDL string is escaped with the builtin python html.escape function before conversion
  2. The systemrdl [quote] and [/quote] constructs are converted to HTML as " rather than "

@coveralls
Copy link

coveralls commented Feb 24, 2025

Coverage Status

coverage: 92.306% (+0.004%) from 92.302%
when pulling 4b4c74e on krcb197:escaping_html
into 3deb51b on SystemRDL:main.

@amykyta3
Copy link
Member

See my comments in parent issue SystemRDL/PeakRDL-html#53
I cannot justify merging this change as-is. Perhaps just the 2nd part for RDLFormatCode [quote] and [/quote] tags.

@krcb197
Copy link
Contributor Author

krcb197 commented Mar 1, 2025

@amykyta3

Thank you for the review comments. I understand that escaping the HTML characters can not be the default behaviour.

I still think it may be helpful in some cases to clean the desc and name properties, I have resubmitted the PR with this as an optional behaviour (off by default). I discovered that the & is escaped by default when the markdown processor is applied.

Does that address your concerns?

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

Successfully merging this pull request may close these issues.

3 participants