Skip to content

Conversation

@florentc
Copy link
Member

@florentc florentc commented Nov 12, 2025

This addresses #654 and #621.

WARNING
This PR adds a db migration that should be applied after #655 is merged (or after editing the migration numbers accordingly).

Comment section

2025-11-12 20-22-50

Error when dismissing without a comment

2025-11-12 20-23-16

Comments are highlighted to easily notice they are non empty

2025-11-12 20-23-57

Comments are saved when status is changed

2025-11-12 20-24-06

Published issues show the comment

2025-11-14 17-33-22

On sec-tracker

2025-11-14 17-33-41

On GitHub (nixpkgs)

2025-11-14 17-34-00

@florentc florentc marked this pull request as ready for review November 17, 2025 10:12
@florentc florentc requested a review from balsoft November 17, 2025 10:13

def additional_comment() -> str:
if comment:
escaped_comment = comment.replace("`", "\\`")

Choose a reason for hiding this comment

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

This will turn ` into

```
\`
```

Which renders as

\`

To escape triple backticks, you need more backticks around the block:

````
```
````

renders as

```

(yes, I added 5 backticks before and after to show this)

Copy link
Member Author

Choose a reason for hiding this comment

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

Solved in dc452ae


# Use at least 3 backticks, or one more than the maximum found in
# order to escape accidents or attempts at escaping the code block
fence_backticks = "`" * max(3, max_backticks + 1)

Choose a reason for hiding this comment

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

This is fine, and I know that fixing a problem with a regex doubles the problem, but iterating over a comment in Python seems scary to me. This could also work to count backtick runs:

max(m.end() - m.start() for m in re.finditer('`+', comment))

Copy link

@YorikSar YorikSar left a comment

Choose a reason for hiding this comment

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

Ah, posted the comment too soon. I'm fine with this implementation as well.

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