getappmap / appmap-js

Client libraries for AppMap
48 stars 17 forks source link

Accept '/' as the command delimiter in addition to '@' #1941

Closed kgilpin closed 1 month ago

kgilpin commented 1 month ago

Currently, our commands such as @help, @plan are triggered by the @ character.

It seems more standard to use / instead.

Refactor the code in the navie project to accept / as the command delimiter, in addition to the current @.

github-actions[bot] commented 1 month ago

Title

Support '/' as a command delimiter in addition to '@'

Problem

Currently, the command delimiter for triggering commands is limited to the @ character. The user wants to also support the / character as a more standard delimiter for commands.

Analysis

The existing implementation parses commands by looking for a prefix with the @ character. To support both @ and / as command delimiters, the logic needs to be modified to handle either character when detecting and parsing commands.

Proposed Changes

  1. packages/navie/src/navie.ts: Modify the command detection logic to support both @ and / characters as command delimiters.

    • Update the command parsing loop to check for both @ and / prefixes.
    • Adjust the slicing operation to account for the detected delimiter.
  2. packages/navie/test: Update or add test cases to verify that both / and @ can be used as command delimiters.

Detailed Steps

  1. packages/navie/src/navie.ts

    • Find the command parsing logic starting around line 152 where commandBuilders are defined and question parsing occurs.
    let { question } = clientRequest;
    question = question.trim();
    
    let command: Command | undefined;
    for (const commandMode of Object.values(CommandMode)) {
      // Check both `@` and `/` prefixes
      const prefixes = [`@${commandMode} `, `/${commandMode} `];
      const linePrefixes = [`@${commandMode}`, `/${commandMode}`];
      for (const prefix of prefixes) {
        if (question.startsWith(prefix) || question.split('\n')[0].trim() === linePrefixes.find(linePrefix => question.startsWith(linePrefix))) {
          command = commandBuilders[commandMode]();
          question = question.slice(prefix.length).trim();
          break;
        }
      }
      if (command) break;
    }
  2. Test Cases

    packages/navie/test/commands:

    • Update existing test cases or add new ones to ensure that both / and @ delimiters are being recognized and processed correctly.
    • For example, modify tests in packages/navie/test/commands/update-command.spec.ts:
    test('correctly parses commands with "@" prefix', () => {
      const request: CommandRequest = {
        question: '@update What is the update?',
        codeSelection: 'some code selection',
        userOptions: new UserOptions(new Map()),
      };
      const result = command.execute(request);
      const updates = await collectUpdates(result);
      expect(updates.length).toBeGreaterThan(0);
    });
    
    test('correctly parses commands with "/" prefix', () => {
      const request: CommandRequest = {
        question: '/update What is the update?',
        codeSelection: 'some code selection',
        userOptions: new UserOptions(new Map()),
      };
      const result = command.execute(request);
      const updates = await collectUpdates(result);
      expect(updates.length).toBeGreaterThan(0);
    });

By making these changes, the system will support both @ and / as valid command delimiters.

kgilpin commented 1 month ago

@dustinbyrne @dividedmind @apotterri your thoughts?

dividedmind commented 1 month ago

I don't think it's a good idea:

kgilpin commented 1 month ago

No plan to implement