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

Added /transcribe requested in issue #35 #53

Closed
wants to merge 1 commit into from

Conversation

HeySreelal
Copy link
Contributor

To specifically transcribe voice messages - especially useful on group chats 🚀

  • Added /transcribe command to specifically ask to transcribe reply_to_message
  • Made the bot do not respond to voice messages in a group chat by default.

Reference: Issue #35

To specifically transcribe voice messages - especially useful on group chats
.gitignore Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@@ -12,6 +12,11 @@ import urlToText from '@/helpers/urlToText'
export default async function handleAudio(ctx: Context) {
try {
const message = ctx.msg

if (message.chat.type == 'group' || message.chat.type == 'supergroup') {
Copy link
Owner

Choose a reason for hiding this comment

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

This shouldn't be the default behaviour, instead we should add a flag on the chat like transcribeAllAudio which is true by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit confused here. So, do we have to add a new command to turn off and on the transcribeAllAudio flag and store it in DB?

@@ -132,8 +138,8 @@ async function sendTranscription(ctx: Context, url: string, fileId: string) {
parse_mode: 'Markdown',
}
)
} catch (error) {
report(error, { ctx, location: 'updateMessagewithError' })
} catch (err) {
Copy link
Owner

Choose a reason for hiding this comment

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

Don't shorten the error variable name :) there is literally no need for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, indeed no need, but just to ignore a warning that they were already defined in upper scope.

Copy link
Owner

Choose a reason for hiding this comment

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

I see. Can we call them sendTranscriptionError in the upper scope then? To avoid confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for sure! I'll change that 🎯

src/handlers/handleAudio.ts Show resolved Hide resolved
@@ -132,8 +138,8 @@ async function sendTranscription(ctx: Context, url: string, fileId: string) {
parse_mode: 'Markdown',
}
)
} catch (error) {
report(error, { ctx, location: 'updateMessagewithError' })
} catch (err) {
Copy link
Owner

Choose a reason for hiding this comment

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

I see. Can we call them sendTranscriptionError in the upper scope then? To avoid confusion

@HeySreelal HeySreelal closed this Apr 27, 2022
@HeySreelal
Copy link
Contributor Author

Sorry for closing this, for now, it was quite a tough time for me. I'll re-open the pull request once I'm back on track.

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

Successfully merging this pull request may close these issues.

2 participants