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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Needle fails to rename thread if longer than 100 chars #173

Closed
MarcusOtter opened this issue May 26, 2022 · 7 comments
Closed

馃悰 Needle fails to rename thread if longer than 100 chars #173

MarcusOtter opened this issue May 26, 2022 · 7 comments
Labels
confirmed bug 馃 Something isn't working

Comments

@MarcusOtter
Copy link
Owner

MarcusOtter commented May 26, 2022

Describe the bug

From Dayhawk#6969 on Discord:

Keep getting this message when trying to use /title, is this because of the update?
image

This is a previously known bug, but I've forgotten to make an issue about it. Discord has a limitation of 100 characters, and we should warn when users try to exceed that limit. We should also make sure to do the same character removal that Discord does, for example they remove /, \, and : (and probably others). Those should be removed before checking the length, and users should be warned that they have been removed.

Steps to reproduce the bug

  1. Run /title with 101 valid characters
  2. See error
  3. Run /title with 100 characters followed by any amount of slashes, backslashes, and colons
  4. Title is still set (as the total amount is 100 valid characters at the end)

Expected behavior

The bot should warn when users try to exceed the 100 character limit

@MarcusOtter MarcusOtter added confirmed bug 馃 Something isn't working good first issue 馃挌 Good for new contributors labels May 26, 2022
@n1ckoates
Copy link
Contributor

We should also make sure to do the same trimming that Discord does, for example they trim /, \, and : (and probably others).

Do they outright remove these characters from thread names, or do they remove them from the start/end?

@MarcusOtter
Copy link
Owner Author

@nchristopher They remove them completely, "trim" was maybe not the best word to describe it

@n1ckoates
Copy link
Contributor

I think we should try...catch the thread rename and return something along the lines of "Something went wrong while renaming the thread, titles cannot be over 100 characters."

@MarcusOtter
Copy link
Owner Author

To not rely on which characters Discord will strip away from the title, you mean? I think that's a reasonable idea as well.

@c43721
Copy link
Contributor

c43721 commented Jul 14, 2022

I think we should try...catch the thread rename and return something along the lines of "Something went wrong while renaming the thread, titles cannot be over 100 characters."

Why do we need to try this when we have access to string.length? I've made a PR for this.

@MarcusOtter
Copy link
Owner Author

@c43721 because Discord strips the invalid characters after we try to set it in the title, so we have know way of knowing how long the length will be after the invalid chars are removed. But yeah, just setting it to 100 character limit is fine imo.

@MarcusOtter
Copy link
Owner Author

Oops, this was closed by #250

@MarcusOtter MarcusOtter removed the good first issue 馃挌 Good for new contributors label Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed bug 馃 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants