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 open parentheses before cite: "(>>1" #527

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

discomrade
Copy link

Allows cite regex to match a post number after an opening parenthesis. "I like the comfy threads (>>1 and >>2) because they're comfy"

From fallenPineapple@d78254b

side note: I believe the other change they did to the regex is logically equivalent, although the other refactoring may be worth considering.

Allows cite regex to match a post number after an opening parenthesis. "I like the comfy threads (>>1 and >>2) because they're comfy"
From fallenPineapple@d78254b
@ctrlcctrlv
Copy link
Member

ctrlcctrlv commented Jan 12, 2023

I'm not likely to ever accept changes to the complex regexes in Vichan†. Someone else would need to take over maintainership burden. The existing ones have proven secure if not in all respects useful. The insane regexes in this software lead to many very subtle ways to create security compromising bugs. If the cite engine was replaced with one that was provably correct and not regex based I might consider that but that's a tall order.

† For non-security reasons.

@ctrlcctrlv
Copy link
Member

ctrlcctrlv commented Jan 12, 2023

Added to README:

3256049

@ctrlcctrlv
Copy link
Member

ctrlcctrlv commented Jan 12, 2023

@basedgentoo
Copy link

I feel like this would be a good addition. In testing it works fine. It's a tiny change, but it's a good one.
I feel that the regex should be reviewed by someone with a deep understanding before it is merged, however.
Also, merging this would let us delete the "unapplied patches" directory and free up some bloat from the README.
I personally don't feel "unapplied patches" should be a thing. That sort of thing should be a configuration options that you can choose from inc/instance-config.php or just not merged because it's decided to be pointless.

@basedgentoo basedgentoo added minor and removed wontfix labels Mar 28, 2023
@ctrlcctrlv
Copy link
Member

Unapplied patches made perfect sense when the project was unmaintained.

@ctrlcctrlv
Copy link
Member

And, I think this is safe but I can't guarantee that obviously. The cites system is insane.

@basedgentoo basedgentoo merged commit 8012cc5 into vichan-devel:master Apr 24, 2023
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.

4 participants