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

Resolves #28: Add change emojis command #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nicoschmdt
Copy link
Collaborator

Adds a command /change_emojis <emojis> which can be used when replying to a sticker. It deletes the sticker from the pack and readds it with the new set of emojis.

@JPTIZ JPTIZ added hacktoberfest-accepted enhancement New feature or request labels Oct 31, 2021
@JPTIZ JPTIZ linked an issue Oct 31, 2021 that may be closed by this pull request
Copy link
Member

@JPTIZ JPTIZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Precisaria de um rebase com o master, pois o commit de user_id pra cor já está no master e está duplicado no PR.

Comment on lines +323 to +329
if msg_type == MsgType.REP_STICKER:
pack_name = msg.reply_to_message.sticker.set_name
sticker_id = msg.reply_to_message.sticker.file_id

if pack_name is None:
msg.reply_text('Não é possível remover o sticker de um pack inexistente.')
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Faltou dizer o que acontece quando a mensagem não é um reply para um sticker. Nesse caso, daria para tratar isso antes de tudo:

Suggested change
if msg_type == MsgType.REP_STICKER:
pack_name = msg.reply_to_message.sticker.set_name
sticker_id = msg.reply_to_message.sticker.file_id
if pack_name is None:
msg.reply_text('Não é possível remover o sticker de um pack inexistente.')
return
if msg_type != MsgType.REP_STICKER:
msg.reply_text(responses.CHANGE_EMOJIS_HELP)
return
pack_name = msg.reply_to_message.sticker.set_name
sticker_id = msg.reply_to_message.sticker.file_id
if pack_name is None:
msg.reply_text(responses.PACK_DOES_NOT_EXIST)
return

Outra coisa é usar o responses para as mensagens de resposta do bot. Coloquei já na sugestão como seria o uso delas, só faltaria adicionar o CHANGE_EMOJIS_HELP e o PACK_DOES_NOT_EXIST no responses.py.

msg = update.message

except Exception as exc:
msg.reply_text("Changing Emojis: Falhou")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
msg.reply_text("Changing Emojis: Falhou")
msg.reply_text(responses.CHANGE_EMOJIS_HELP)

Comment on lines +352 to +354
if msg_type not in [MsgType.STICKER, MsgType.REP_STICKER]:
update.message.reply_text("Message is not a reply to a sticker.")
return
Copy link
Member

@JPTIZ JPTIZ Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aí, com as mudanças de cima isso já seria feito no início., então o if aqui seria desnecessário. Acho que daria para deixar essa verificação antes do try ainda.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concordo.
Outro detalhe, por consistência as mensagens de resposta pros usuários são sempre em português (ou deveriam ser, pelo menos hehe).


bot.delete_sticker_from_set(sticker_id)
# add_sticker(bot, update)
msg = update.message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isso já é feito na primeira linha deste método.

if len(splittext) > 1:
emoji = splittext[1]
else:
emoji = DEFAULT_EMOJI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eu acho que não faz muito sentido ter um emoji default para este comando. Se o usuário não informar nenhum emoji seria melhor apenas retornar uma mensagem de erro.
O "autocomplete" do Telegram pra comandos de bot faz com que o comando seja enviado imediatamente, sem o usuário poder completar a mensagem com os "argumentos", então o emoji default iria mais atrapalhar do que ajudar neste caso.

Comment on lines +352 to +354
if msg_type not in [MsgType.STICKER, MsgType.REP_STICKER]:
update.message.reply_text("Message is not a reply to a sticker.")
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concordo.
Outro detalhe, por consistência as mensagens de resposta pros usuários são sempre em português (ou deveriam ser, pelo menos hehe).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Criar comando /change_emojis - Create command /change_emojis
3 participants