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

Allow markup-ing literal strings #3402

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Allow markup-ing literal strings #3402

merged 1 commit into from
Jun 25, 2024

Conversation

xmo-odoo
Copy link
Contributor

@xmo-odoo xmo-odoo commented Jun 14, 2024

Literal strings in the application should be safe (similar to static markup in template files), and the normal way to create dynamic markup code side: create a properly marked up Markup, then Markup.format user-defined content into it.

@CLAassistant
Copy link

CLAassistant commented Jun 14, 2024

CLA assistant check
All committers have signed the CLA.

@xmo-odoo
Copy link
Contributor Author

Will need to squash the PR in order to use the PR's description as commit message: had a typo ("and then normal way")

@xmo-odoo
Copy link
Contributor Author

CI seems unhappy, even though 3.9.2 does seem available in the python-versions and #3404 seems to have fetched it fine.

Possibly a temporary network issue?

@xmo-odoo xmo-odoo changed the base branch from main to develop June 24, 2024 08:01
@xmo-odoo
Copy link
Contributor Author

xmo-odoo commented Jun 24, 2024

Aaah I think I see what the issue is, I forked off of and targeted the main branch, not sure how (since I just used github's base UI).

develop seems to be the default and correct branch to target. I retargeted the PR and rebased, and updated the commit message to fix the typo while at it. Hopefully it works better like this.

Literal strings in the application should be safe (similar to static
markup in template files), and the normal way to create dynamic markup
code side: create a properly marked up `Markup`, then `Markup.format`
user-defined content into it.
@xmo-odoo
Copy link
Contributor Author

xmo-odoo commented Jun 25, 2024

Right, yep, turns out using github's interface led me to not test the rule and thus forget half of it.

Also wow does the test suite not joke around. But since I was running it I added a few test cases to make sure the change actually works, probably can't hurt. Hopefully I got it right this time.

@p4p3r p4p3r merged commit 4ccd3b9 into semgrep:develop Jun 25, 2024
8 checks passed
@xmo-odoo xmo-odoo deleted the patch-1 branch June 26, 2024 05:49
@xmo-odoo
Copy link
Contributor Author

Thanks 🎆

@p4p3r
Copy link
Collaborator

p4p3r commented Jun 26, 2024

Thanks for your contribution! Your changes will be available in Semgrep’s registry in a bit.

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.

4 participants