Tweak add and remove output

Rather than outputting all at the end, output as we go.  Also return the
actual error when adding/removing, not the error when trying to output.

Author: code-asher

cli/add.go

@@ -66,41 +66,36 @@ func add() *cobra.Command {
 				isDir = stat.IsDir()
 			}
 
-			var summary []string
 			var failed []string
 			if isDir {
 				files, err := os.ReadDir(args[0])
 				if err != nil {
 					return err
 				}
 				for _, file := range files {
-					vsixPath := filepath.Join(args[0], file.Name())
-					_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Adding %s...\r\n", vsixPath)
-					s, err := doAdd(ctx, vsixPath, store)
+					s, err := doAdd(ctx, filepath.Join(args[0], file.Name()), store)
 					if err != nil {
+						_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Failed to unpack %s: %s\n", file.Name(), err.Error())
 						failed = append(failed, file.Name())
-						summary = append(summary, fmt.Sprintf("Failed to unpack %s: %s", file.Name(), err.Error()))
 					} else {
-						summary = append(summary, s...)
+						_, _ = fmt.Fprintln(cmd.OutOrStdout(), strings.Join(s, "\n"))
 					}
 				}
 			} else {
-				_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Adding %s...\r\n", args[0])
-				summary, err = doAdd(ctx, args[0], store)
+				s, err := doAdd(ctx, args[0], store)
 				if err != nil {
 					return err
 				}
+				_, _ = fmt.Fprintln(cmd.OutOrStdout(), strings.Join(s, "\n"))
 			}
 
-			_, err = fmt.Fprintln(cmd.OutOrStdout(), strings.Join(summary, "\n"))
-			failedCount := len(failed)
-			if failedCount > 0 {
+			if len(failed) > 0 {
 				return xerrors.Errorf(
 					"Failed to add %s: %s",
-					util.Plural(failedCount, "extension", ""),
+					util.Plural(len(failed), "extension", ""),
 					strings.Join(failed, ", "))
 			}
-			return err
+			return nil
 		},
 	}
 

cli/add_test.go

@@ -155,7 +155,9 @@ func TestAdd(t *testing.T) {
 					require.Regexp(t, test.error, err.Error())
 				} else {
 					require.NoError(t, err)
+					require.NotContains(t, output, "Failed to add")
 				}
+
 				// Should list all the extensions that worked.
 				for _, ext := range test.extensions {
 					// Should exist on disk.

cli/remove.go

@@ -97,17 +97,25 @@ func remove() *cobra.Command {
 				return xerrors.Errorf("%s does not exist", targetId)
 			}
 
-			summary := []string{fmt.Sprintf("Removed %s", util.Plural(len(toDelete), "version", ""))}
+			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Removing %s...\n", util.Plural(len(toDelete), "version", ""))
+			var failed []string
 			for _, delete := range toDelete {
 				err = store.RemoveExtension(ctx, publisher, name, delete)
 				if err != nil {
-					summary = append(summary, fmt.Sprintf("  - %s (%s)", delete, err))
+					_, _ = fmt.Fprintf(cmd.OutOrStdout(), "  - %s (%s)\n", delete, err)
+					failed = append(failed, delete.String())
 				} else {
-					summary = append(summary, fmt.Sprintf("  - %s", delete))
+					_, _ = fmt.Fprintf(cmd.OutOrStdout(), "  - %s\n", delete)
 				}
 			}
-			_, err = fmt.Fprintln(cmd.OutOrStdout(), strings.Join(summary, "\n"))
-			return err
+
+			if len(failed) > 0 {
+				return xerrors.Errorf(
+					"Failed to remove %s: %s",
+					util.Plural(len(failed), "version", ""),
+					strings.Join(failed, ", "))
+			}
+			return nil
 		},
 	}
 

cli/remove_test.go

@@ -35,10 +35,9 @@ func TestRemove(t *testing.T) {
 	tests := []struct {
 		// all means to pass --all.
 		all bool
-		// error is the expected error.
+		// error is the expected error, if any.
 		error string
-		// expected contains the versions should have been deleted.  It is ignored
-		// in the case of an expected error.
+		// expected contains the versions should have been deleted, if any.
 		expected []storage.Version
 		// extension is the extension to remove.  Every version of
 		// testutil.Extensions[0] will be added before each test.
@@ -170,8 +169,17 @@ func TestRemove(t *testing.T) {
 				require.Regexp(t, test.error, err.Error())
 			} else {
 				require.NoError(t, err)
-				require.Contains(t, output, fmt.Sprintf("Removed %d version", len(test.expected)))
+				require.NotContains(t, output, "Failed to remove")
+			}
+
+			// Should list all the extensions that were able to be removed.
+			if len(test.expected) > 0 {
+				require.Contains(t, output, fmt.Sprintf("Removing %d version", len(test.expected)))
 				for _, version := range test.expected {
+					// Should not exist on disk.
+					dest := filepath.Join(extdir, test.extension.Publisher, test.extension.Name, version.String())
+					_, err := os.Stat(dest)
+					require.Error(t, err)
 					require.Contains(t, output, fmt.Sprintf("  - %s\n", version))
 				}
 			}