Skip to content

content [nfc]: Pull out parseUserMention, give regexp less to do #1086

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

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Nov 25, 2024

This gives us more breathing room for making this handling more
complicated, as we'll need for the new "channel-wildcard-mention"
class (#1064) and for distinguishing different types of mentions (#646, #647).

Prompted by:
#1073 (comment)
(/cc @sm-sayedi, @PIG208).

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Nov 25, 2024
Copy link
Member

@PIG208 PIG208 left a comment

Choose a reason for hiding this comment

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

Left one comment. Overall this looks good to me! Feel free to merge after fixing it.

}

if (i != classes.length) {
// There was some class we didn't expect.
Copy link
Member

Choose a reason for hiding this comment

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

nit: "class" -> "classes"

Copy link
Member Author

Choose a reason for hiding this comment

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

The singular is intentional: there's at least one class we didn't expect, namely the one at position i.

(Grammatically it'd be either "was some class" or "were some classes".)

@PIG208 PIG208 added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Dec 3, 2024
@gnprice
Copy link
Member Author

gnprice commented Dec 3, 2024

Thanks for the review! Merging.

This gives us more breathing room for making this handling more
complicated, as we'll need for the new "channel-wildcard-mention"
class (zulip#1064).
This should further help make space for adding more logic here,
both for the "channel-wildcard-mention" class (zulip#1064) and for
distinguishing different types of mentions (zulip#646, zulip#647).
@gnprice gnprice merged commit d21d76b into zulip:main Dec 3, 2024
1 check passed
@gnprice gnprice deleted the pr-parse-mention branch December 3, 2024 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants