search

dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.37k stars 378 forks source link

dotnet-suggest register --help lists --suggestion-command, which has no effect #1772

Open KalleOlaviNiemitalo opened 2 years ago

KalleOlaviNiemitalo commented 2 years ago

The output of dotnet-suggest register --help describes this option:

  --suggestion-command <suggestion-command>  The command to invoke to retrieve suggestions

However, the option has no effect. The feature was removed in https://github.com/dotnet/command-line-api/commit/b4c9bf30de996fd6f69fbbcacd5af49f12965e5a#diff-db24858dec500aba0043fcb1c98e223575175909c47519bdeadaf164634be993L121-R123. Please remove it from the --help output as well, or change its description to state that it has no effect.

jonsequitur commented 2 years ago

It's used here:

https://github.com/dotnet/command-line-api/blob/de20229747f5a7ecd14df18ac6d547fbe462e524/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs#L232-L236

KalleOlaviNiemitalo commented 2 years ago

The --suggestion-command option is added when invoking dotnet-suggest register, but it is not used by dotnet-suggest register itself. So to preserve compatibility, dotnet-suggest register will still have to accept the option, but the --help output should not claim that it does something. That claim caused me to spend time trying to reregister the command with different values of --suggestion-command when tab completion did not work.

KalleOlaviNiemitalo commented 2 years ago

Besides, what's the idea behind passing currentProcessFileNameWithoutExtension as --suggestion-command? That does not include the directory name, so if dotnet-suggest register actually saved that string and later attempted to invoke it as a command to retrieve suggestions, it would require the command to be on PATH.

jonsequitur commented 2 years ago

I see. Yes, it probably needs to be left in for backwards compatibility for now.

Besides, what's the idea behind passing currentProcessFileNameWithoutExtension as --suggestion-command? That does not include the directory name, so if dotnet-suggest register actually saved that string and later attempted to invoke it as a command to retrieve suggestions, it would require the command to be on PATH.

It was designed to work with global commands and is in need of revisiting because that leads to bugs like the one you mentioned here. The developer loop wasn't given much attention for this feature and the design could use updating.