apply suggestions from code review

Author: umbynos

Diff for: cli/arguments/completion.go

@@ -16,7 +16,7 @@ import (
 // It returns a list of fqbn
 // it's taken from cli/board/listall.go
 func GetInstalledBoards() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 
 	list, _ := board.ListAll(context.Background(), &rpc.BoardListAllRequest{
 		Instance:            inst,
@@ -34,24 +34,25 @@ func GetInstalledBoards() []string {
 // GetInstalledProtocols is an helper function useful to autocomplete.
 // It returns a list of protocols available based on the installed boards
 func GetInstalledProtocols() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 	pm := commands.GetPackageManager(inst.Id)
 	boards := pm.InstalledBoards()
 
-	// use this strange map because it should be more optimized
-	// we use only the key and not the value because we do not need it
-	intalledProtocolsMap := make(map[string]struct{})
+	installedProtocols := make(map[string]struct{})
 	for _, board := range boards {
-		// we filter and elaborate a bit the informations present in Properties
 		for _, protocol := range board.Properties.SubTree("upload.tool").FirstLevelKeys() {
-			if protocol != "default" { // remove this value since it's the default one
-				intalledProtocolsMap[protocol] = struct{}{}
+			if protocol == "default" {
+				// default is used as fallback when trying to upload to a board
+				// using a protocol not defined for it, it's useless showing it
+				// in autocompletion
+				continue
 			}
+			installedProtocols[protocol] = struct{}{}
 		}
 	}
-	res := make([]string, len(intalledProtocolsMap))
+	res := make([]string, len(installedProtocols))
 	i := 0
-	for k := range intalledProtocolsMap {
+	for k := range installedProtocols {
 		res[i] = k
 		i++
 	}
@@ -61,7 +62,7 @@ func GetInstalledProtocols() []string {
 // GetInstalledProgrammers is an helper function useful to autocomplete.
 // It returns a list of programmers available based on the installed boards
 func GetInstalledProgrammers() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 	pm := commands.GetPackageManager(inst.Id)
 
 	// we need the list of the available fqbn in order to get the list of the programmers
@@ -72,8 +73,8 @@ func GetInstalledProgrammers() []string {
 	})
 
 	installedProgrammers := make(map[string]string)
-	for _, i := range list.Boards {
-		fqbn, _ := cores.ParseFQBN(i.Fqbn)
+	for _, board := range list.Boards {
+		fqbn, _ := cores.ParseFQBN(board.Fqbn)
 		_, boardPlatform, _, _, _, _ := pm.ResolveFQBN(fqbn)
 		for programmerID, programmer := range boardPlatform.Programmers {
 			installedProgrammers[programmerID] = programmer.Name
@@ -92,7 +93,7 @@ func GetInstalledProgrammers() []string {
 // GetUninstallableCores is an helper function useful to autocomplete.
 // It returns a list of cores which can be uninstalled
 func GetUninstallableCores() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 
 	platforms, _ := core.GetPlatforms(&rpc.PlatformListRequest{
 		Instance:      inst,
@@ -110,7 +111,7 @@ func GetUninstallableCores() []string {
 // GetInstallableCores is an helper function useful to autocomplete.
 // It returns a list of cores which can be installed/downloaded
 func GetInstallableCores() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 
 	platforms, _ := core.PlatformSearch(&rpc.PlatformSearchRequest{
 		Instance:    inst,
@@ -125,13 +126,23 @@ func GetInstallableCores() []string {
 	return res
 }
 
-// GetUninstallableLibs is an helper function useful to autocomplete.
+// GetInstalledLibraries is an helper function useful to autocomplete.
+// It returns a list of libs which are currently installed, including the builtin ones
+func GetInstalledLibraries() []string {
+	return getLibraries(true)
+}
+
+// GetUninstallableLibraries is an helper function useful to autocomplete.
 // It returns a list of libs which can be uninstalled
-func GetUninstallableLibs() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+func GetUninstallableLibraries() []string {
+	return getLibraries(false)
+}
+
+func getLibraries(all bool) []string {
+	inst := instance.CreateAndInit()
 	libs, _ := lib.LibraryList(context.Background(), &rpc.LibraryListRequest{
 		Instance:  inst,
-		All:       false,
+		All:       all,
 		Updatable: false,
 		Name:      "",
 		Fqbn:      "",
@@ -147,7 +158,7 @@ func GetUninstallableLibs() []string {
 // GetInstallableLibs is an helper function useful to autocomplete.
 // It returns a list of libs which can be installed/downloaded
 func GetInstallableLibs() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 
 	libs, _ := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchRequest{
 		Instance: inst,
@@ -165,7 +176,7 @@ func GetInstallableLibs() []string {
 // It returns a list of boards which are currently connected
 // Obviously it does not suggests network ports because of the timeout
 func GetConnectedBoards() []string {
-	inst := instance.CreateAndInit() // TODO optimize this: it does not make sense to create an instance everytime
+	inst := instance.CreateAndInit()
 
 	list, _ := board.List(&rpc.BoardListRequest{
 		Instance: inst,

Diff for: cli/cli.go

@@ -105,18 +105,20 @@ func createCliCommandTree(cmd *cobra.Command) {
 	cmd.AddCommand(version.NewCommand())
 
 	cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, tr("Print the logs on the standard output."))
-	cmd.PersistentFlags().String("log-level", "", tr("Messages with this level and above will be logged. Valid levels are: %s", "trace, debug, info, warn, error, fatal, panic"))
+	validLogLevels := []string{"trace", "debug", "info", "warn", "error", "fatal", "panic"}
+	cmd.PersistentFlags().String("log-level", "", tr("Messages with this level and above will be logged. Valid levels are: %s", strings.Join(validLogLevels, ", ")))
 	cmd.RegisterFlagCompletionFunc("log-level", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return []string{"trace", "debug", "info", "warn", "error", "fatal", "panic"}, cobra.ShellCompDirectiveDefault
+		return validLogLevels, cobra.ShellCompDirectiveDefault
 	})
 	cmd.PersistentFlags().String("log-file", "", tr("Path to the file where logs will be written."))
-	cmd.PersistentFlags().String("log-format", "", tr("The output format for the logs, can be: %s", "text, json"))
+	validFormats := []string{"text", "json"}
+	cmd.PersistentFlags().String("log-format", "", tr("The output format for the logs, can be: %s", strings.Join(validFormats, ", ")))
 	cmd.RegisterFlagCompletionFunc("log-format", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return []string{"text", "json"}, cobra.ShellCompDirectiveDefault
+		return validFormats, cobra.ShellCompDirectiveDefault
 	})
-	cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", tr("The output format for the logs, can be: %s", "text, json"))
+	cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", tr("The output format for the logs, can be: %s", strings.Join(validFormats, ", ")))
 	cmd.RegisterFlagCompletionFunc("format", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return []string{"text", "json"}, cobra.ShellCompDirectiveDefault
+		return validFormats, cobra.ShellCompDirectiveDefault
 	})
 	cmd.PersistentFlags().StringVar(&configFile, "config-file", "", tr("The custom config file (if not specified the default will be used)."))
 	cmd.PersistentFlags().StringSlice("additional-urls", []string{}, tr("Comma-separated list of additional URLs for the Boards Manager."))

Diff for: cli/lib/check_deps.go

@@ -41,7 +41,7 @@ func initDepsCommand() *cobra.Command {
 		Args: cobra.ExactArgs(1),
 		Run:  runDepsCommand,
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-			return arguments.GetUninstallableLibs(), cobra.ShellCompDirectiveDefault
+			return arguments.GetInstalledLibraries(), cobra.ShellCompDirectiveDefault
 		},
 	}
 	return depsCommand

Diff for: cli/lib/examples.go

@@ -43,7 +43,7 @@ func initExamplesCommand() *cobra.Command {
 		Args:    cobra.MaximumNArgs(1),
 		Run:     runExamplesCommand,
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-			return arguments.GetUninstallableLibs(), cobra.ShellCompDirectiveDefault
+			return arguments.GetInstalledLibraries(), cobra.ShellCompDirectiveDefault
 		},
 	}
 	examplesCommand.Flags().StringVarP(&examplesFlags.fqbn, "fqbn", "b", "", tr("Show libraries for the specified board FQBN."))

Diff for: cli/lib/uninstall.go

@@ -40,7 +40,7 @@ func initUninstallCommand() *cobra.Command {
 		Args:    cobra.MinimumNArgs(1),
 		Run:     runUninstallCommand,
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-			return arguments.GetUninstallableLibs(), cobra.ShellCompDirectiveDefault
+			return arguments.GetUninstallableLibraries(), cobra.ShellCompDirectiveDefault
 		},
 	}
 	return uninstallCommand

Diff for: i18n/data/en.po

@@ -365,7 +365,7 @@ msgstr "Checksum:"
 msgid "Clean caches."
 msgstr "Clean caches."
 
-#: cli/cli.go:122
+#: cli/cli.go:124
 msgid "Comma-separated list of additional URLs for the Boards Manager."
 msgstr "Comma-separated list of additional URLs for the Boards Manager."
 
@@ -1214,7 +1214,7 @@ msgstr "Internal error in cache"
 msgid "Invalid '%[1]s' property: %[2]s"
 msgstr "Invalid '%[1]s' property: %[2]s"
 
-#: cli/cli.go:263
+#: cli/cli.go:265
 msgid "Invalid Call : should show Help, but it is available only in TEXT mode."
 msgstr "Invalid Call : should show Help, but it is available only in TEXT mode."
 
@@ -1271,11 +1271,11 @@ msgstr "Invalid library"
 msgid "Invalid network.proxy '%[1]s': %[2]s"
 msgstr "Invalid network.proxy '%[1]s': %[2]s"
 
-#: cli/cli.go:224
+#: cli/cli.go:226
 msgid "Invalid option for --log-level: %s"
 msgstr "Invalid option for --log-level: %s"
 
-#: cli/cli.go:241
+#: cli/cli.go:243
 msgid "Invalid output format: %s"
 msgstr "Invalid output format: %s"
 
@@ -1444,7 +1444,7 @@ msgstr "Maintainer: %s"
 msgid "Max time to wait for port discovery, e.g.: 30s, 1m"
 msgstr "Max time to wait for port discovery, e.g.: 30s, 1m"
 
-#: cli/cli.go:108
+#: cli/cli.go:109
 msgid "Messages with this level and above will be logged. Valid levels are: %s"
 msgstr "Messages with this level and above will be logged. Valid levels are: %s"
 
@@ -1667,7 +1667,7 @@ msgstr "Package website:"
 msgid "Paragraph: %s"
 msgstr "Paragraph: %s"
 
-#: cli/cli.go:112
+#: cli/cli.go:113
 msgid "Path to the file where logs will be written."
 msgstr "Path to the file where logs will be written."
 
@@ -2044,7 +2044,7 @@ msgstr "The connected devices search timeout, raise it if your board doesn't sho
 msgid "The connected devices search timeout, raise it if your board doesn't show up e.g.: 10s"
 msgstr "The connected devices search timeout, raise it if your board doesn't show up e.g.: 10s"
 
-#: cli/cli.go:121
+#: cli/cli.go:123
 msgid "The custom config file (if not specified the default will be used)."
 msgstr "The custom config file (if not specified the default will be used)."
 
@@ -2064,8 +2064,8 @@ msgid "The key '%[1]v' is not a list of items, can't remove from it.\n"
 msgstr "The key '%[1]v' is not a list of items, can't remove from it.\n"
 "Maybe use '%[2]s'?"
 
-#: cli/cli.go:113
-#: cli/cli.go:117
+#: cli/cli.go:115
+#: cli/cli.go:119
 msgid "The output format for the logs, can be: %s"
 msgstr "The output format for the logs, can be: %s"
 
@@ -2147,7 +2147,7 @@ msgstr "Unable to get Local App Data Folder: %v"
 msgid "Unable to get user home dir: %v"
 msgstr "Unable to get user home dir: %v"
 
-#: cli/cli.go:210
+#: cli/cli.go:212
 msgid "Unable to open file for logging: %s"
 msgstr "Unable to open file for logging: %s"