Skip to content

Conversation

@Alidra
Copy link

@Alidra Alidra commented Aug 28, 2025

Description

Checklist:

@Alidra Alidra requested a review from a team as a code owner August 28, 2025 13:37
Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

See inline comments.

The samples/Lambdapi/List.lp sample is also too big. If it's suppressed in the diff, it's too big for our needs. Please replace it.

Can you please update the search query in the PR template to more accurately identify just this langauge. From a quick look it appears to be matching other languages already correctly identified. You can use regular expressions so I'd recommend using the one you're adding for the heuristic, once you've addressed the problems with it.

@Alidra
Copy link
Author

Alidra commented Sep 24, 2025

See inline comments.

The samples/Lambdapi/List.lp sample is also too big. If it's suppressed in the diff, it's too big for our needs. Please replace it.

Can you please update the search query in the PR template to more accurately identify just this langauge. From a quick look it appears to be matching other languages already correctly identified. You can use regular expressions so I'd recommend using the one you're adding for the heuristic, once you've addressed the problems with it.

I believe all the comments have been addressed. Please let me know if any more changes are required.

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

See inline comment.

@Alidra Alidra requested a review from lildude October 1, 2025 11:25
@Alidra
Copy link
Author

Alidra commented Oct 23, 2025

Hi @lildude, Are there any more changes requested to merge this PR?

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

Your heuristic is vulnerable to ReDoS. We require all regular expressions to be linear, safe from ReDoS and RE2 compatible. Please fix this.

Once fixed, this PR won't be merged until our popularity requirement is met. This is checked when a new release is made and I do not comment if the usage still hasn't been met.

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