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

[enhancement] add callback to conversation forms #117

Closed
zxsleebu opened this issue Sep 19, 2024 · 6 comments · Fixed by #122
Closed

[enhancement] add callback to conversation forms #117

zxsleebu opened this issue Sep 19, 2024 · 6 comments · Fixed by #122
Labels
enhancement New feature or request v2 Needs to be fixed before v2

Comments

@zxsleebu
Copy link

zxsleebu commented Sep 19, 2024

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
Copy link
Member

KnorpelSenf commented Sep 19, 2024

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
Copy link
Author

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
Copy link
Member

Please go ahead!

@KnorpelSenf KnorpelSenf added the enhancement New feature or request label Sep 19, 2024
@zxsleebu zxsleebu changed the title [enhancement] auto delete messages in forms [enhancement] add callback to conversations forms Sep 19, 2024
@zxsleebu zxsleebu changed the title [enhancement] add callback to conversations forms [enhancement] add callback to conversation forms Sep 19, 2024
@KnorpelSenf
Copy link
Member

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
Copy link
Author

zxsleebu commented Sep 24, 2024

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
Copy link
Member

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.

@KnorpelSenf KnorpelSenf added the v2 Needs to be fixed before v2 label Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2 Needs to be fixed before v2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants