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

autocomplete: Add support for composing "silent mentions". #3393

Closed
wants to merge 1 commit into from

Conversation

ungps
Copy link

@ungps ungps commented Mar 11, 2019

Add markdown syntax for "silent mentions", which happens when
a user starts their mention with "@_". Add empty "silent" class
for future styling changes.

There is also a PR with some changes on zulip-markdown-parser repo. zulip/zulip-markdown-parser#1

Issue: #3374

Add markdown syntax for "silent mentions", which happens when
a user starts their mention with "@_". Add empty "silent" class
for future styling changes.
@gnprice
Copy link
Member

gnprice commented Mar 20, 2019

Thanks @ungps !


   const filter: string =
     text.length > lastIndex + 1 && !['\n', ' '].includes(text[lastIndex + 1])
-      ? text.substring(lastIndex + 1, text.length)
+      ? text.substring(
+          lastIndex + 1 + (text[lastIndex] === '@' && text[lastIndex + 1] === '_'),
+          text.length,
+        )
       : '';

Oof, this is pretty complicated. 😩

This code is already too complicated -- see #3345 for discussion. It's already hard to understand, and I think that's hiding some bugs already. It gets significantly more so with this change.

I'm going to declare a moratorium on adding features to this code without making it simpler to compensate. @AB261 is working on that right now in #3390. If you can make small local changes to this spot to keep the complexity down, we can move forward on this independent of #3390; otherwise, let's wait for that work to play out.

Separately:

+.silent {
+}

This has no effect and isn't necessary; so just leave it out.

@gnprice
Copy link
Member

gnprice commented Apr 19, 2019

Closing, for the reasons above. @ungps , thanks again, and please feel free to send a new PR, now or later, as described here:

If you can make small local changes to this spot to keep the complexity down, we can move forward on this independent of #3390; otherwise, let's wait for that work to play out.

@gnprice gnprice closed this Apr 19, 2019
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.

2 participants