grammyjs / conversations

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

[enhancement] add callback to conversation forms #117

Closed zxsleebu closed 1 week ago

zxsleebu commented 1 month ago

for example:

const value = await conversation.form.text({ action: ctx => ctx.deleteMessage(), otherwise: () => {...} })
//message should be deleted here

it's useful because form functions don't return anything except values. thanks @KnorpelSenf for the idea

i'd like it a lot more than

const { msg: msg1 } = await conversation.waitFor(":text")
await msg1?.delete();
... use msg1.text ...
// ...
const { msg: msg2 } = await conversation.waitFor(":text")
await msg2?.delete();
... use msg2.text ...
KnorpelSenf commented 1 month ago

Wouldn't it be better to just provide a callback so people can do this?

const value = await conversation.form.text({ action: ctx => ctx.deleteMessage(), otherwise: () => {...} })

Then it isn't limited to deletions only, but messages can also be copied elsewhere or reacted to or whatever.

zxsleebu commented 1 month ago

Wouldn't it be better to just provide a callback so people can do this?

const value = await conversation.form.text({ action: ctx => ctx.deleteMessage(), otherwise: () => {...} })

Then it isn't limited to deletions only, but messages can also be copied elsewhere or reacted to or whatever.

sounds and looks a lot better!

if you don't resist, i will replace my idea with this one in the starting message

KnorpelSenf commented 1 month ago

Please go ahead!

KnorpelSenf commented 1 month ago

Naturally, it would make sense to implement all form utilities around a new generic form building helper function.

const text: string = await conversation.form.build({
  select: ctx => ctx.has(":text") ? { ok: true, value: ctx.msg.text } : { ok: false },
  action: ctx => ctx.deleteMessage(),
  otherwise: ctx => ctx.reply("no text"),
})

All current form implementations of the plugin can then be based on this abstraction. In addition, it permits people to build their own forms utilities on top of it. Having such a base makes it trivial to add the requested action callback, as it simply needs to be passed on.

zxsleebu commented 1 month ago

that's an interesting idea, but forms get rid of their simplicity. maybe it would better if the build function returns the function. so we will be able to use it again and again. and it is important to still have an ability to overwrite its options by passing them as in the builder. also i don't really get why do we have to return the whole object { ok: true, value: ctx.msg.text } if we can simplify it and return only the result.

const getText: (options: FormOptions) => Promise<string> = conversation.form.build({
  select: ctx => ctx.has(":text") && ctx.msg.text,
  action: ctx => ctx.deleteMessage(),
  otherwise: ctx => ctx.reply("no text"),
})
const text1 = await getText();
const text2 = await getText({ action: ctx => {} });

also we still need prebuilt functions as text, photo, etc...

KnorpelSenf commented 1 month ago

that's an interesting idea, but forms get rid of their simplicity.

No. They will not be changed. This is purely additive. If the API is not good enough to be exposed, then the method can be made private. Either way, it will radically simplify the implementation of forms.

maybe it would better if the build function returns the function. so we will be able to use it again and again. and it is important to still have an ability to overwrite its options by passing them as in the builder.

This can already be done today. We do not need to modify the plugin for that.

also i don't really get why do we have to return the whole object { ok: true, value: ctx.msg.text } if we can simplify it and return only the result.

That is because forms perform both validation and selection. The result value has to encode failure and success, and upon success, a value. These types do that.