limactl help and info flag should show available plugins #4009

olamilekan000 · 2025-09-09T10:22:02Z

change adds functionality to display Plugins in the info and list commands

Fixes #3608 (comment)

website/content/en/docs/config/plugin/cli.md

pkg/plugin/plugin.go

website/content/en/docs/config/plugin/cli.md

jandubois

I haven't looked at the docs changes, but here is some feedback on the code.

cmd/limactl/main.go

pkg/limainfo/limainfo.go

pkg/plugin/plugin.go

jandubois

Docs review

website/content/en/docs/config/plugin/cli.md

website/content/en/docs/releases/experimental.md

AkihiroSuda · 2025-09-17T09:26:48Z

BATS failing

ok 38 setting both allow list and block list generates a warning in 138ms
not ok 39 limactl info includes the default block list in 124ms
# (in test file hack/bats/tests/preserve-env.bats, line 152)
#   `run -0 limactl yq '.shellEnvBlock[]' <<<"$output"' failed, expected exit code 0, got 1
# time="2025-09-17T09:21:44Z" level=warning msg="Plugin discovery: failed to scan directory /snap/bin: open /snap/bin: no such file or directory"
# time="2025-09-17T09:21:44Z" level=warning msg="Plugin discovery: failed to scan directory /home/runner/.local/bin: open /home/runner/.local/bin: no such file or directory"
# time="2025-09-17T09:21:44Z" level=warning msg="Plugin discovery: failed to scan directory /home/runner/.config/composer/vendor/bin: open /home/runner/.config/composer/vendor/bin: no such file or directory"
# time="2025-09-17T09:21:44Z" level=warning msg="Plugin discovery: failed to scan directory /snap/bin: open /snap/bin: no such file or directory"
# Error: bad file '-': yaml: mapping values are not allowed in this context
# ===== protect =====
ok 40 create dummy instance in 66ms

https://github.com/lima-vm/lima/actions/runs/17775630264/job/50573056648?pr=4009

pkg/plugin/plugin.go

Signed-off-by: olalekan odukoya <[email protected]>

jandubois

Looks mostly fine, but I still have some stylistic feedback, and some minor issues.

jandubois · 2025-09-19T02:45:27Z

cmd/limactl/main.go

@@ -51,7 +51,7 @@ func main() {
 	rootCmd := newApp()
 	if err := executeWithPluginSupport(rootCmd, os.Args[1:]); err != nil {
 		server.StopAllExternalDrivers()
-		handleExitError(err)
+		usrlocalsharelima.HandleExitError(err)


usrlocalsharelima is kind of an unexpected place for this function. I would move it maybe to its own file in pkg/osutil/exit.go.

But this is nitpicking, unless there are other reasons to update the PR once more, this can also be fixed up later.

jandubois · 2025-09-19T02:50:26Z

cmd/limactl/main.go

@@ -51,7 +51,7 @@ func main() {
 	rootCmd := newApp()


Nit-picking: This whole block could be written like this:

rootCmd := newApp() err := executeWithPluginSupport(rootCmd, os.Args[1:]) server.StopAllExternalDrivers() usrlocalsharelima.HandleExitError(err) if err != nil { logrus.Fatal(err) }

jandubois · 2025-09-19T02:55:02Z

cmd/limactl/main.go

+	if err := rootCmd.ParseFlags(args); err == nil {
+		if debug, _ := rootCmd.Flags().GetBool("debug"); debug {
+			logrus.SetLevel(logrus.DebugLevel)
+			debugutil.Debug = true
 		}
 	}


It bothers me that this code block exists in 2 places. Can't it be called early enough that it covers both execution branches?

I think this here might be enough, and you can delete the copy in the PersistentPreRun command?

jandubois · 2025-09-19T02:59:09Z

pkg/plugin/plugin.go

+	var dirs []string
+
+	selfDirs := usrlocalsharelima.SelfDirs()
+	dirs = append(dirs, selfDirs...)


This could be just

Suggested change

var dirs []string

selfDirs := usrlocalsharelima.SelfDirs()

dirs = append(dirs, selfDirs...)

dirs := usrlocalsharelima.SelfDirs()

jandubois · 2025-09-19T03:01:20Z

pkg/plugin/plugin.go

+	dirs := getPluginDirectories()
+
+	for _, dir := range dirs {
+		pluginsInDir := scanDirectory(dir)
+		for _, plugin := range pluginsInDir {


This may be a personal preference, but I prefer to avoid temp variables that are only used once, unless they are necessary to keep the expressions short. But I find this easier to read:

Suggested change

dirs := getPluginDirectories()

for _, dir := range dirs {

pluginsInDir := scanDirectory(dir)

for _, plugin := range pluginsInDir {

for _, dir := range getPluginDirectories() {

for _, plugin := range scanDirectory(dir) {

Of course this only works because the functions don't return an error.

jandubois · 2025-09-19T03:31:07Z

pkg/plugin/plugin.go

+	if runtime.GOOS == "windows" {
+		ext := filepath.Ext(path)
+		return isWindowsExecutableExt(ext)
+	}


I'm not sure if this code is needed.

The file extension is only needed when you run a command without an extension. E.g. when you run tar Windows will try each extension from PATHEXT in turn to find an executable: tar.exe, tar.bat, etc.

But Windows will execute any command that exists as-is, regardless of the extension:

C:\Users\suse>tar tar: Must specify one of -c, -r, -t, -u, -x C:\Users\suse>copy C:\Windows\System32\tar.exe hello.world 1 file(s) copied. C:\Users\suse>hello.world hello.world: Must specify one of -c, -r, -t, -u, -x

So I think I would just remove this block completely. We want to strip the common extensions from the plugin name for the --help output, but here we don't really care.

jandubois · 2025-09-19T03:35:44Z

pkg/plugin/plugin.go

+	}
+
+	if !strings.HasPrefix(string(content), "#!") {
+		logrus.Debugf("Plugin %s: not a script file, skipping description extraction", filepath.Base(path))


I think the debug log should always include the full path name. This is for debugging issues, normal users will not see the output because they don't use --debug.

Suggested change

logrus.Debugf("Plugin %s: not a script file, skipping description extraction", filepath.Base(path))

logrus.Debugf("Plugin %s: not a script file, skipping description extraction", path)

Same for the next error below.

jandubois · 2025-09-19T03:39:38Z

pkg/plugin/plugin.go

+	var descs []string
+	for _, match := range matches {
+		if len(match) > 1 {
+			descs = append(descs, strings.TrimSpace(match[1]))
+		}
+	}
+
+	if len(descs) == 0 {
+		return ""
+	}
+
+	mergedDesc := descs[0]
+	for _, line := range descs[1:] {
+		mergedDesc += "\n" + strings.Repeat(" ", 22) + line
+	}
+
+	logrus.Debugf("Plugin %s: extracted merged description from script:\n%q", filepath.Base(path), mergedDesc)
+	return mergedDesc


I think you should ignore everything but the first match. And you definitely should not include multi-line formatting in the description here; that would have to be done at the presentation layer. But I think we should simply recommend to use short, one-line descriptions.

So the whole block should be just return matches[0] and that's it.

jandubois · 2025-09-19T03:48:06Z

website/content/en/docs/releases/experimental.md

+- `limactl plugins` plugin mechanism (see [CLI plugins](../config/plugin/cli))
+  - `limactl-*`


This line should be in the previous block ("The following features are experimental and subject to change:") under the "External drivers:" line, and look like this:

Suggested change

- `limactl plugins` plugin mechanism (see [CLI plugins](../config/plugin/cli))

- `limactl-*`

- `limactl plugins:` plugin mechanism (see [CLI plugins](../config/plugin/cli))

There are no specific commands to list down here because plugins would be invoked just like a builtin command, and we can't list limactl * here. 😄

jandubois · 2025-09-19T03:50:29Z

website/content/en/docs/config/plugin/cli.md

+```
+
+**Format Requirements:**
+- Only files beginning with a shebang (`#!`) are treated as scripts, and their <limactl-desc> lines will be extracted as the plugin description i.e Must contain exactly `<limactl-desc>Description text</limactl-desc>`


Tag needs to be quoted here:

Suggested change

- Only files beginning with a shebang (`#!`) are treated as scripts, and their <limactl-desc> lines will be extracted as the plugin description i.e Must contain exactly `<limactl-desc>Description text</limactl-desc>`

- Only files beginning with a shebang (`#!`) are treated as scripts, and their `<limactl-desc>` lines will be extracted as the plugin description i.e Must contain exactly `<limactl-desc>Description text</limactl-desc>`

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch 3 times, most recently from 9993a6e to e012aa1 Compare September 9, 2025 11:30

olamilekan000 mentioned this pull request Sep 9, 2025

limactl help and info flag should show available plugins #3973

Closed

AkihiroSuda added this to the v2.0.0 milestone Sep 9, 2025

AkihiroSuda added the area/cli limactl CLI user experience label Sep 9, 2025

AkihiroSuda reviewed Sep 9, 2025

View reviewed changes

website/content/en/docs/config/plugin/cli.md Show resolved Hide resolved

AkihiroSuda reviewed Sep 9, 2025

View reviewed changes

pkg/plugin/plugin.go Show resolved Hide resolved

AkihiroSuda reviewed Sep 9, 2025

View reviewed changes

website/content/en/docs/config/plugin/cli.md Outdated Show resolved Hide resolved

AkihiroSuda reviewed Sep 9, 2025

View reviewed changes

website/content/en/docs/config/plugin/cli.md Show resolved Hide resolved

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch from e012aa1 to 6854c86 Compare September 15, 2025 00:08

olamilekan000 requested review from AkihiroSuda and jandubois September 15, 2025 00:08

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch from 6854c86 to 51673df Compare September 15, 2025 00:12

jandubois requested changes Sep 15, 2025

View reviewed changes

jandubois requested changes Sep 16, 2025

View reviewed changes

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch 2 times, most recently from f2fea51 to 2d073bc Compare September 16, 2025 18:35

olamilekan000 requested a review from jandubois September 16, 2025 18:38

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch from 2d073bc to ff781c8 Compare September 16, 2025 18:38

jandubois reviewed Sep 17, 2025

View reviewed changes

pkg/plugin/plugin.go Outdated Show resolved Hide resolved

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch from ff781c8 to 5a7b1fa Compare September 19, 2025 01:11

limactl help and info flag should show available plugins

79ef8e3

Signed-off-by: olalekan odukoya <[email protected]>

olamilekan000 force-pushed the show-available-plugins-for-info-and-exec-cmd branch from 5a7b1fa to 79ef8e3 Compare September 19, 2025 02:08

jandubois requested changes Sep 19, 2025

View reviewed changes

	logrus.Debugf("Plugin %s: not a script file, skipping description extraction", filepath.Base(path))
	logrus.Debugf("Plugin %s: not a script file, skipping description extraction", path)

		- `limactl plugins` plugin mechanism (see [CLI plugins](../config/plugin/cli))
		- `limactl-*`

	- `limactl plugins` plugin mechanism (see [CLI plugins](../config/plugin/cli))
	- `limactl-*`
	- `limactl plugins:` plugin mechanism (see [CLI plugins](../config/plugin/cli))

	- Only files beginning with a shebang (`#!`) are treated as scripts, and their <limactl-desc> lines will be extracted as the plugin description i.e Must contain exactly `<limactl-desc>Description text</limactl-desc>`
	- Only files beginning with a shebang (`#!`) are treated as scripts, and their `<limactl-desc>` lines will be extracted as the plugin description i.e Must contain exactly `<limactl-desc>Description text</limactl-desc>`

limactl help and info flag should show available plugins #4009

Are you sure you want to change the base?

limactl help and info flag should show available plugins #4009

Uh oh!

Conversation

olamilekan000 commented Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AkihiroSuda commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AkihiroSuda commented Sep 17, 2025 •

edited

Loading