grammyjs / commands Goto Github PK

Currently the main use case for getNearestCommand is to work as a 404 for messages containing unknown. commands
Example:

// after all our command registration
bot
   .filter(Context.has.filterQuery('::bot_command'))
   .use(async (ctx) => {
      const suggestedCommand = ctx.getNearestCommand(myCommands)
   })

A command registered with a custom prefix, would never trigger the filter ::bot_command, like +doX

As me and @roziscoding talked, it's needed to overwrite the ctx.entities("bot_command") method for one that hydrates with not only commands entities, but also commands registered with custom prefixes

A ready to use 404 function could also be useful, e.g:

// after all our command registration
bot.use(
   command404(
      'did you mean $COMMAND',
      'msg for not found',
   ).localize('es', 'quisiste decir $COMMAND', 'comando no encontrado'),
)

Right now fuzzyMatch is searching trough all localized and non-localized versions of the commands.
Example mimicking the inner workings of the fuzzyMatch:

const cmds = new Commands<Context>();
            cmds.command('butcher', 'green beret', () => { })
            cmds.command('duke', 'sniper', () => { }).localize('es', 'duque', 'francotirador')
            cmds.command('fins', 'marine', () => { }).addToScope({ type: "all_private_chats" }, () => { })
            
            const commandsSet = new Set(
                cmds
                    .toArgs()
                    .flatMap((item) => item.commands.map((command) => command.command)),
            );

returns:

Set(4) { "butcher", "duke", "fins", "duque" }

This is important since fuzzyMatch could have matched against a localized/default command version that it's not relevant to the user lang, in other words, returned another foreign local of the command, in any case, one that the end-user is not seeing from the commands menu.
It's basically a collision problem.

A more appropriate search would be to filter only trough the respective ctx.from?.language.scope and opt-out for search-in-all instead.
Must work when no localization exist for a command.
Same thing could be said for the botCommandScope, but that might be irrelevant.

Edit: I have a PoC branch at improveFuzzySearch f782af6 but it's a little bit of a mess. I think I coupled it too much with the changes exposing the prefix (that's needed for correctly recommend the command + it's respective prefix).

Do:

• Test's that demonstrate the issue in a copy of the main branch. see below
• Since from.language.scope it's in IETF language tag, should add a function to assert when it's ISO-639, or convert when possible to ISO-639, if not, fallback to "default". Depends on grammyjs/types#45 approval
• Lower similarity threshold to 0.82

It would be cool if the plugin supported exporting and importing commands. This feature would enhance the modularity and reusability of command definitions, making it easier to manage and maintain large bot projects.

Something like this:

1. Exporting a Command:

import { Command } from "@grammyjs/commands";

const startCommand = new Command("start", "Initializes bot configuration")
    .addToScope(
        { type: "all_group_chats" },
        (ctx) => ctx.reply(`Hello, members of ${ctx.chat.title}!`),
    );

export default startCommand;

2. Importing a Command

import { Bot } from "grammy";
import { Commands } from "@grammyjs/commands";
import startCommand from "./commands/start.js";

const bot = new Bot("<telegram token>");

const myCommands = new Commands();

myCommands.define(startCommand);

// Calls `setMyCommands`
await myCommands.setCommands(bot);

// Registers the command handlers
bot.use(myCommands);

bot.start();

*Here maybe the .define could support both single Command or an array of Command (?)

Benefits:

• Modularity: Commands can be organized in separate files, making the project structure cleaner and easier to manage.
• Reusability: Common commands can be reused across different projects by simply importing them.
• Maintainability: Changes to a specific command can be made in its dedicated file, reducing the risk of conflicts and making the codebase easier to maintain.

for the following command group:

const userCommands = new Commands()

userCommands.command('start', 'example', (ctx) => {
    ctx.reply('a')
})
userCommands.command('entecito', '_', () => {}, {
    prefix: '?',
})

calling either:

await userCommands.setCommands(bot)

bot.command('help', async (ctx) => {
    await ctx.setMyCommands(userCommands)
})

will register

when it should only register the example start command

Even if RegExp commands are not meant to be used / listed in the commands menu, a more appropriate error should be shown. Noted that if an error handler is not registered, it will stop execution

In my opinion it should be ok to show regexp commands in the commands menu, even if they are not meant to be used in their string representation. It should not be a problem since their names are already being registered as strings

myCommands.command(/delete_(.*)/, 'el otro', async (c) => {
   await c.reply('papito')
})

await myCommands.setCommands(bot)

will throw:

error: Uncaught (in promise) GrammyError: Call to 'setMyCommands' failed! (400: Bad Request: BOT_COMMAND_INVALID)
    return new GrammyError(
           ^
    at toGrammyError (https://deno.land/x/[email protected]/core/error.ts:43:12)
    at ApiClient.callApi (https://deno.land/x/[email protected]/core/client.ts:328:20)
    at eventLoopTick (ext:core/01_core.js:168:7)
    at async Promise.all (index 0)
    at async Commands.setCommands (file:///media/martincito/freedom/repos/grammy/commands/src/commands.ts:197:9)
    at async file:///media/martincito/freedom/repos/grammy/testbot-deno/src/bot.ts:62:1

Even a regex-like-string will throw:

userCommands.command('/delete/', 'el otro', async (c) => {
   await c.reply('papito')
})

Since the plugin allows to organize commands into multiple and diverse Commands instances, would make sense to allow

getNearestCommand: (
        commands: Commands<C>,
        options?: Partial<JaroWinklerOptions>,
    ) => string | null

to search across a Commands array. Internally should be adjusted for that too.
In any case this could be kept as it is since it's possible to do something in the lines of:

bot
.filter(Context.has.filterQuery("::bot_command"))
.use(async (ctx) => {
 let suggestedCommand
 for(const commands of [userCommands, adminCommands]){
   const search = ctx.getNearestCommand(commands)
   if(search){
     suggestedCommand = search 
   }
 }

 if (suggestedCommand) {
   return ctx.reply(
     `Hmm... I don't know that command. Did you mean ${suggestedCommand}?`,
   );
 }
 await ctx.reply("Oops... I don't know that command :/");
});

But this is not ideal since fuzzySearch does not expose its returned value similarityThreshold. That could be exposed and let users do whatever logic they want or managed internally and return the most similar command.
Also prefix of the best match should be exposed too

Roz quote:

in setMyCommands, we could:

• throw a nicer error at the user
• add an ignoreInvalidCommands flag

I think there should be an option to make command names case-insensitive, or do it by default.

Use Case

There are many use cases for this, and here's an example. Imagine we have a bot that configures PC builds, and includes a footer like this in one of its messages:

See /available_CPUs.

This cannot be handled if the command name passed to the filter doesn't look like the one in the message. And once it's handled the way it is displayed to the user, it can't be handled when the cases are changed, say all lower like /available_cpus.

It is case-insensitive by default in most of the other bot frameworks.

With a sample bot using something in the lines:

const userCommands = new Commands<MyContext>();
bot.use(commands());

userCommands.command("start", "init bot", (c) => {
  c.reply("started");
}).localize("es", "iniciar", "Inicializa el bot")
  .addToScope(
    { type: "all_private_chats" },
    (ctx) => ctx.reply(`iniciado`),
  );

userCommands.command("end", "end", (c) => {
  c.reply("end");
}).localize("es", "fin", "finaliza")
  .addToScope(
    { type: "all_private_chats" },
    (ctx) => ctx.reply(`finalizado`),
  );

throws:

[
  {
    scope: { type: "chat", chat_id: <ID> },
    language_code: undefined,
    commands: [
      { command: "start", description: "init bot" },
      { command: "end", description: "end" }
    ]
  },
  {
    scope: { type: "chat", chat_id: <ID> },
    language_code: "es",
    commands: [
      { command: "", description: "Inicializa el bot" },
      { command: "", description: "finaliza" }
    ]
  }
]
Error in middleware while handling update 286148219 GrammyError: Call to 'setMyCommands' failed! (400: Bad Request: command must be non-empty)

Most likely due to probably this line inside the toObject utility:

The line responsible for selecting the text to be search against while looking for similar command names makes too much assumptions about the user input and dev preferences regarding what is a command and where it's located

• the prefix is always 1 character long.
• the command is always at the start.
• the user input only contains a command, and nothing else.

this assumptions are contradictory with the plugin features that:

• allow for arbitrary length custom prefixes
• allow for matching commands at any position

and contradictory with the user unexpected behavior of typing:

• '/myCommand and some text'

this could be mitigated making use of the proposed addition of hydrated command entities in #31