MusicBot icon indicating copy to clipboard operation
MusicBot copied to clipboard

Fix plural-s for cmd-clean-reply and cmd-remove-reply

Open rec0de opened this issue 2 years ago • 4 comments

After creating your pull request, tick these boxes if they are applicable to you.

  • [x] I have tested my changes against the review branch (the latest developmental version), and this pull request is targeting that branch as a base
  • [x] I have tested my changes on Python 3.5.3 or higher

Description

Fixes the logic that determines if a plural "s" is added to item/items or message/messages in cmd-remove-reply and cmd-clean-reply, previously producing messages like "Cleaned up 0 message".

Somewhat related, I feel like the "item(s)" in cmd-remove-reply (l. 2944, entry_text) should be part of the translation string such that only the "s" is added as needed, like for cmd-clean-reply. This would allow for better translations that include custom translations for the word "item" but would also require changes to all existing translation files.

If that sounds reasonable to you, I could add the change here and adjust all translation files to produce the same output as before, from where they could be individually updated to provide an actual translation of "item".

Cheers!

Related issues (if applicable)

rec0de avatar Sep 15 '21 15:09 rec0de

Looks like a good change to me, all translation files are done by volunteers however and as far as I know we have no sure fire way of communication with them it would be up to users to adjust the translation files as needed, I do think you should add it though, could you also please run black over any changes you make? python -m pip install black then python -m black {file/s}

AutumnClove avatar Sep 15 '21 17:09 AutumnClove

Sure can, I'll change this to a draft for now

rec0de avatar Sep 15 '21 17:09 rec0de

Seeing as you're already making changes, I think perhaps we could additionally add some logic for if no messages are deleted, something like if len(deleted) < 1: "No messages have been cleared" of course if you don't feel up to that, that's fine, just a suggestion.

AutumnClove avatar Sep 15 '21 18:09 AutumnClove

Hmm, I personally feel like that wouldn't be worth the added complexity — especially since you'd still need the "s" check, and it would 'break' existing translations. So I think I'll leave it at that for now, but thanks for the suggestion :)

The last commit I added should not change visible behaviour at all. I've only tested the English version, but the change is basically the same in all files so there shouldn't be any issues.

rec0de avatar Sep 15 '21 19:09 rec0de