Caesar cipher command#421
Conversation
purefunctor
added
type: feature
ks129
left a comment
ks129
left a comment
There was a problem hiding this comment.
Quick review without actual testing.
kwzrd
left a comment
kwzrd
left a comment
There was a problem hiding this comment.
Looks good and works fine apart from the path problem. I haven't looked at the actual shift algorithm you have implemented in much detail yet, but it looks interesting. Defining the resource dict such that an embed can be built from it directly is a really cool idea.
ghost
added
the
status: waiting for author
ghost
added
needs 2 approvals
and removed
status: waiting for author
Den4200
left a comment
Den4200
left a comment
There was a problem hiding this comment.
It all looks good overall, but I have not tested this yet.
ghost
added
status: waiting for author
ghost
added
needs 2 approvals
and removed
status: waiting for author
| def caesar_func(text: str) -> Iterable[str]: | ||
| """Implements a lazy Caesar Cipher algorithm.""" | ||
| for char in text: | ||
| if not char.isascii() or not char.isalpha() or char.isspace(): | ||
| yield char | ||
| continue | ||
|
|
||
| case_start = 65 if char.isupper() else 97 | ||
| true_offset = (ord(char) - case_start + offset) % 26 | ||
|
|
||
| yield chr(case_start + true_offset) | ||
|
|
||
| def conversion_func(text: str) -> str: | ||
| """Encrypts the given string using the Caesar Cipher.""" | ||
| return "".join(caesar_func(text)) |
There was a problem hiding this comment.
Is there a reason why these are defined inside the command? From the look of it, you can move it outside as well to prevent them being defined everytime the command is run, declaring them as @staticmethod
There was a problem hiding this comment.
I've initially decided to define these inside of the function to be consistent with the form of the other commands inside of the Fun cog. I could change them into @staticmethods but I'm not sure whether making them standalone top-level functions would avoid polluting the cog.
There was a problem hiding this comment.
I would suggest moving them outside, in case we want to write unittests for these. Naming these with a _ in front of the name to signify that these are to be used internally might be able to help, otherwise you can always move them outside in their own class and just call it inside the Cog.
There was a problem hiding this comment.
I'll go ahead and do that 👍
This makes caesar_func a top-level function and renames it to caesar_cipher.
This changes the converters used by caesarcipher_encrypt and caesarcipher_decrypt in order to accomodate for the manual conversion that _get_text_and_embed does, which allows for this feature to be easily disabled.
7 participants
Relevant Issues
Closes #420.
Description
This adds a
.caesarciphercommand group withdecrypt,encrypt, andinfosubcommands in the Fun cog. Thedecryptandencryptsubcommands also support message IDs and links similarly to other Fun cog commands such as.uwuand.randomcase.Reasoning
The
decryptandencryptsubcommands should provide a concise way of using the ciphering a message, withinfoadding good educational value to the feature.Screenshots
Additional Details
In order to support message IDs and links for the command, the
_caesar_ciphermethod was patterned against the uwu and random case commands. Maybe this feature can be abstracted to a generalized function to avoid redundancy/for future text-related commands as well?Did you:
pipenv lock?pipenv run lint)?