microsoft / inshellisense

IDE style command line auto complete
MIT License
8.21k stars 179 forks source link

fix: subCommands find problem #179

Closed life2015 closed 3 months ago

life2015 commented 4 months ago

First of all, thanks for this awesome project, which has given me a lot of inspiration in the areas of terminal experience and smart completion. I have recently been trying to integrate it with Xterm.js. However, I encountered some issues while using it and reading the code.

What is the problem ?

When I type npm run, the terminal autocomplete returns the incorrect result "--registry", instead of the scripts in package.json or other run parameters.

image

This behavior is somewhat strange, so I delved into the source code to look for the issue.

After spending some time debugging, the problem was finally located in the genSubCommand method of the runtime.ts file.

image

Here, we can see that when typing npm run, function genSubCommand attempting to find s.name === 'run'within ·parentCommand.subcommands· is unsuccessful, returning subcommandIdx is -1.

However, the judgment logic in the code is SubCommand == null, which obviously does not apply when subCommand is -1. As a result, it proceeds to parentCommand.subcommands.at(-1), effectively retrieving the last item in the subcommands list (the 69th item). Therefore, the output becomes --registry instead of the correct result.

image

How to Fix

image

By inspecting the contents of parentCommand.subcommands through the Debugger, it can be observed that the name of a subCommand might be an array of strings. And the type of subCommand also prove this.

image

Therefore, we need to fix the find matching logic for subCommands to handle both arrays and strings during the match.

  const subcommandIdx = parentCommand.subcommands?.findIndex((s) => {
    // subCommand.name can be a string or an array of strings
    if (Array.isArray(s.name)) {
      // if it's an array, we check if the command is included in the array
      return s.name.includes(command);
    } else {
      return s.name === command;
    }
  });

Additionally, we also need to optimize the verification logic before and after the find match to handle exceptional cases.

  if (!parentCommand.subcommands || parentCommand.subcommands.length === 0) return;
  // ... find logic 
  if (subcommandIdx === -1) return;

After fix

After fix, npm run looks great ! image

life2015 commented 4 months ago

@cpendery

life2015 commented 3 months ago

Amazing fix, thank you for the detailed write up and PR!! Just a few minor nits and it looks good to go!

Great patch! Just applied these changes!