grammyjs / conversations

Conversational interfaces for grammY.
https://grammy.dev/plugins/conversations
MIT License
51 stars 17 forks source link

feat: `conversation.halt` #53

Closed KnorpelSenf closed 1 week ago

KnorpelSenf commented 2 years ago

A lot of people ask for a way to exit conversations without returning or throwing. Similar to process.exit, we should add conversation.halt which stops the conversation immediately.

Calling ctx.conversation.exit from within a conversation doesn't work well because the conversation continues to the next wait or skip, which is undesirable and not possible to avoid.

It should be called halt or terminate not exit in order to avoid confusion.

await conversation.halt(); // never resolves
yn4v4s commented 2 years ago

So, if we do:

...
ctx = await conversation.waitFor(
  <anything_but_text>,
  (ctx) => ctx.message?.text === "/cancel" && conversation.halt()
);
...

What happens to this update? Does it get dropped? Or does it continue to be processed by the other middlewares (like in conversation.skip()). Or should we do: conversation.halt().then(() => conversation.skip({ drop: true })) like in #43

KnorpelSenf commented 2 years ago

I would say that halt should drop the update by default. But now that you say it, I do agree that there's a use case for halting the conversation, and giving back control to the middleware system. So perhaps we should rather do

ctx = await conversation.waitUnless(
  Context.has.command("cancel"),
  () => conversation.halt()
)

to end the conversation and handle it in the midleware system, and use

ctx = await conversation.waitUnless(
  Context.has.command("cancel"),
  () => conversation.halt({ drop: true })
)

to consume the update?

yn4v4s commented 2 years ago

Yup! LGTM

yn4v4s commented 2 years ago

Or maybe consuming the update should be the default, as you said; then you need to specify drop: false to give back control to the middleware system?

KnorpelSenf commented 2 years ago

That would be the same interface as for skip but with a different default value. That would be pretty confusing, what do you think? 🤔

yn4v4s commented 2 years ago

Setting defaults to true in these cases feels more natural to me, but I agree with you, they should be consistent between the two. I'm ok with either one, true for both, or false for both.

KnorpelSenf commented 2 years ago

We cannot change the default value for skip because that would be a breaking change. The current behaviour of skip (passing updates back the middleware system) is necessary to support parallel conversations, i.e. having several conversations in the same chat with different people.

But alright, then halt will have to be implemented with false as the default value, and permit to drop the most recent update using { drop: true }.

Thanks for your input! 🎉

yn4v4s commented 1 year ago

Yo, I have been very busy with work. What's the status of this one? Is there something that needs to be tested/reviewed?

KnorpelSenf commented 1 year ago

Not yet, it still remains to be implemented. Do you want to work on this?

hamedf62 commented 1 year ago

hi, what if user leave the conversation by selecting any other commands int he middle of running a conversation? how to detect if any other command choose by user meanwhile?

KnorpelSenf commented 1 year ago

You can install the command handlers before the respective conversation using bot.command. If you don't call next in them, the conversation won't be reached. See https://grammy.dev/guide/middleware

KnightNiwrem commented 6 months ago

Adding https://t.me/grammyjs/235195 as a point of consideration.

Given potential use cases where one might try to call halt or exit inside the conversation function, but through a plugin; it raises some question on whether halt can naively explode.

KnorpelSenf commented 6 months ago

Yep, calling halt from inside run should be supported