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

Parsing error of channel names with multiple dashes #24

Closed
shmulvad opened this issue Dec 15, 2022 · 6 comments · Fixed by #39
Closed

Parsing error of channel names with multiple dashes #24

shmulvad opened this issue Dec 15, 2022 · 6 comments · Fixed by #39
Assignees

Comments

@shmulvad
Copy link

One of my Slack channels was named something similar to string1---string2. This got parsed as string1-string2 when creating the channel, but as string1---string2 when trying to post messages to the channel (which then failed because a channel with that name hadn't yet been created). This should be fixed so it is consistent.

@richfromm
Copy link
Owner

Apologies for taking so long to respond to this. I didn't see any of these open issues until yesterday. See #22 (comment)

I am able to locally reproduce the issue, see attached log.

slack2discord.log

@richfromm
Copy link
Owner

I can't find anything documenting it (certainly not in the create_text_channel() docs), but empirically it looks like Discord doesn't allow channels to have multiple dashes.

This is even outside of the API, doing it manually within the GUI, I can't type in a second dash in the popup for entering the name for a new channel. It just doesn't recognize that I've typed that character.

But the API behavior is a little weird. Rather than raise an error if you pass an invalid name, it instead just converts multiple dashes into a single dash, creates the new channel with that modified name, then returns that name to you as the new channel. And that causes an assertion of mine to fail, since it doesn't match what's expected:

assert channel.name == channel_name, f"New channel has unexpected name: {channel.name}"
. That seems like a really bad idea to me.

I suppose we could just go along with this, and automatically do the same, but I don't like the idea of doing that. What if you have both foo-bar and foo--bar, what should we do then? I think I'd rather just detect this situation and alert the user if there are channels with illegal names, and fail early (with a suitable error message), and let the user resolve this. Esp. since there is already a mechanism for having different channel names in Discord compared to Slack, via the --channel-file option.

It also appears that empirically Discord doesn't allow spaces in channels, so there's the question as to whether we should include that in the channel name validation or not. Their treatment of spaces is a little different, if you type one in the GUI it actually translates that to a dash instead. (I haven't verified if the same thing happens with the API, but I'd guess so.) And then a second space just doesn't work, just like a second dash doesn't work.

However, it appears that Slack also doesn't allow for spaces, so maybe this isn't worth worrying about in practice? I'm undecided, but I might just toss it in anyway, since if I'm going to add some channel name validation and an error msg to inform the user about how to deal with it, it's pretty trivial to add one more criteria to the validation check.

@richfromm
Copy link
Owner

Allegedly there is no limit on channel name length in Discord (unlike Slack), see https://discord.com/moderation/208-channel-categories-and-names#title-2

So I think I will limit validation to checking for repeated dashes, as well as any spaces (or perhaps any whitespace).

UPDATE: In the Discord GUI, trying every non-alphanumeric character on my (US) keyboard, the only non-alphanumeric characters accepted are dash (-) and underscore (_). This is consistent with Slack, but I can't find that documented anywhere for Discord. I will do some experimenting and see if the API is consistent with the GUI or not, regardless of whatever the documentation does not say.

@richfromm
Copy link
Owner

richfromm commented Feb 8, 2023

I did some tests regarding channel creation. It would be nice if this were just documented. Here's what I found regarding channel names:

  • alphanumeric allowed
  • single dash allowed
  • underscore allowed
  • multiple dashes are converted into a single dash
  • space is converted to dash
  • tab is removed from name
  • newline is removed from name
  • tilde is coverted to a dash
  • all other symbols are silently removed
  • channel name must be between 1 and 100 characters (inclusive) in length

An additional observation is that if you try to create a channel with a name that already exists, that succeeds, a new channel is created with a dupe name. I don't want to put too much energy into dealing with this, b/c IMHO it's a really stupid thing to do. Regardless, right now in DiscordClient.get_channel() I'm assuming that this can't happen, and if it does I log an error and raise RuntimeError. Perhaps a better strategy would be to just pick the first one found and log a warning. Esp. if we end up making the default behavior to prompt the user to review the channel situation before proceeding, see #23 (comment)

Anyway, given all of the various undocumented restrictions, I think my validation criteria will be:

  • alphanumeric plus dash plus underscore are the only allowed characters
  • no multiple dashes allowed
  • name length must be <= 100

UPDATE: I hadn't explicitly tested for this before, but double underscore is fine, as is underscore followed by dash, and dash followed by underscore. It's only multiple dashes in a row that are a problem (even dash underscore dash is fine).

@richfromm
Copy link
Owner

See my current plan here

@richfromm
Copy link
Owner

richfromm commented Feb 15, 2023

Somewhat change of plan. I still like the idea of querying Discord state early, and (by default) prompting the user for confirmation regarding the category/channel situation, as outlined above. And I will try to get to it in the not too distant future.

But, for reasons discussed earlier, it's non-trivial to implement, given the async nature of the Discord API.

So for the short term resolution of this issue, I think it's fine to validate all of the supplied Discord channel names (and fail fast if any do not), early in the process, without querying Discord state to see whether or not the channels exist.

Similar short term change of plan at #23 (comment)

@richfromm richfromm self-assigned this Feb 20, 2023
richfromm added a commit that referenced this issue Feb 21, 2023
The following criteria must be met:
* Names must contain only alphanumeric, dash (-), and/or underscore (_) characters
* Length must be between 1 and 100 chars (inclusive)
* Multiple dashes in a row (e.g. --) are not permitted

The actual reality of Discord channel creation is a bit more complex. Empirically, there
are numerous (undocumented?) cases where an input channel name does **not** fail to create
a channel, but the channel created does not have precisely the same name as the input. For
example:
* Multiple dashes in a row are converted into a single dash
* Space and tilde are converted to dash
* Tab and newline are removed
* All other symbols are removed

For our purposes, this is too complex to deal with, and we will fail to validate any input
that does not result in creating a channel with the same name as the input.

#24
@richfromm richfromm linked a pull request Feb 21, 2023 that will close this issue
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 a pull request may close this issue.

2 participants