From b1434fac4130a630db54d2fe42d15f2415b19376 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Wed, 11 Nov 2026 10:03:13 -0800 Subject: [PATCH] ls: remove trailing padding without changing columns --- cmd/format_test.go | 2 +- cmd/listing_e2e_test.go | 76 +++++++++++++++++++++++++++++++++++++++++ cmd/ls.go | 30 ++++++++-------- cmd/ls_test.go | 28 ++++----------- cmd/root_test.go | 3 ++ 5 files changed, 101 insertions(+), 38 deletions(-) create mode 100644 cmd/listing_e2e_test.go diff --git a/cmd/format_test.go b/cmd/format_test.go index 172d494c..2227b93d 100644 --- a/cmd/format_test.go +++ b/cmd/format_test.go @@ -174,7 +174,7 @@ func TestFormatFileMetadataWithOptsShort(t *testing.T) { } got := formatFileMetadataWithOpts(meta, listOptions{long: false}) - want := "/test.txt\t" + want := "/test.txt" if got != want { t.Errorf("short format = %q, want %q", got, want) } diff --git a/cmd/listing_e2e_test.go b/cmd/listing_e2e_test.go new file mode 100644 index 00000000..14f77c71 --- /dev/null +++ b/cmd/listing_e2e_test.go @@ -0,0 +1,76 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" +) + +const listingOutputE2EEnv = "DBXCLI_TEST_LISTING_OUTPUT_FIXTURE" + +func installListingOutputFixture(t *testing.T) { + t.Helper() + stubUsersClient(t, &mockUsersClient{}) + stubFilesClient(t, &mockFilesClient{ + listFolderFn: func(*files.ListFolderArg) (*files.ListFolderResult, error) { + return &files.ListFolderResult{Entries: []files.IsMetadata{ + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/one"}}, + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/two words"}}, + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/three.zip"}}, + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/four"}}, + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/five"}}, + }}, nil + }, + searchV2Fn: func(*files.SearchV2Arg) (*files.SearchV2Result, error) { + return files.NewSearchV2Result([]*files.SearchMatchV2{ + searchMatch(&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/zeta"}}), + searchMatch(&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/alpha two"}}), + }, false), nil + }, + }) +} + +func TestListingTextOutputEndToEnd(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + {"ls", []string{"ls"}, "/one /two words /three.zip /four\n/five\n"}, + {"search", []string{"search", "query"}, "/zeta\n/alpha two\n"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + code, stdout, stderr := executeExitTestSubprocess(t, tt.args, map[string]string{ + envAccessToken: "test-token", + listingOutputE2EEnv: "1", + }) + if code != 0 || stderr != "" { + t.Fatalf("exit code = %d, stderr = %q", code, stderr) + } + if stdout != tt.want { + t.Fatalf("output = %q, want %q", stdout, tt.want) + } + }) + } +} + +func TestListingLongOutputHasNoTrailingPaddingEndToEnd(t *testing.T) { + for _, args := range [][]string{{"ls", "-l"}, {"search", "-l", "query"}} { + t.Run(args[0], func(t *testing.T) { + code, stdout, stderr := executeExitTestSubprocess(t, args, map[string]string{ + envAccessToken: "test-token", + listingOutputE2EEnv: "1", + }) + if code != 0 || stderr != "" { + t.Fatalf("exit code = %d, stderr = %q", code, stderr) + } + for _, line := range strings.Split(strings.TrimSuffix(stdout, "\n"), "\n") { + if strings.TrimRight(line, " \t") != line { + t.Fatalf("line has trailing padding: %q", line) + } + } + }) + } +} diff --git a/cmd/ls.go b/cmd/ls.go index 44020d7e..38278b15 100644 --- a/cmd/ls.go +++ b/cmd/ls.go @@ -65,7 +65,7 @@ func printFolderMetadata(w io.Writer, e *files.FolderMetadata, longFormat bool) } func formatFolderMetadata(e *files.FolderMetadata, longFormat bool) string { - text := fmt.Sprintf("%s\t", e.PathDisplay) + text := e.PathDisplay if longFormat { text = "-\t-\t-\t" + text } @@ -77,7 +77,7 @@ func formatFileMetadata(e *files.FileMetadata, longFormat bool) string { } func formatFileMetadataWithOpts(e *files.FileMetadata, opts listOptions) string { - text := fmt.Sprintf("%s\t", e.PathDisplay) + text := e.PathDisplay if opts.long { t := getTime(e, opts) text = fmt.Sprintf("%s\t%s\t%s\t", e.Rev, humanize.IBytes(e.Size), formatTime(t, opts)) + text @@ -86,7 +86,7 @@ func formatFileMetadataWithOpts(e *files.FileMetadata, opts listOptions) string } func formatDeletedMetadata(e *files.DeletedMetadata, longFormat bool) string { - text := fmt.Sprintf("%s\t", e.PathDisplay) + text := e.PathDisplay if longFormat { text = "-\t-\t-\t" + text } @@ -345,11 +345,15 @@ func renderLsResults(out io.Writer, entries []files.IsMetadata, opts listOptions w.Init(out, 4, 8, 1, ' ', 0) itemCounter := 0 printItem := func(message string) { - itemCounter = itemCounter + 1 - _, _ = fmt.Fprint(w, message) - if (itemCounter%4 == 0) || opts.long { - _, _ = fmt.Fprintln(w) + if itemCounter > 0 { + if itemCounter%4 == 0 || opts.long { + _, _ = fmt.Fprintln(w) + } else { + _, _ = fmt.Fprint(w, "\t") + } } + _, _ = fmt.Fprint(w, message) + itemCounter++ } if opts.long { @@ -367,7 +371,10 @@ func renderLsResults(out io.Writer, entries []files.IsMetadata, opts listOptions } } - return finishListOutput(w, itemCounter, opts) + if itemCounter > 0 { + _, _ = fmt.Fprintln(w) + } + return w.Flush() } func isListFolderNotFolderError(err error) bool { @@ -386,13 +393,6 @@ func isListRevisionsNotFileError(err error) bool { apiErr.EndpointError.Path.Tag == files.LookupErrorNotFile } -func finishListOutput(w *tabwriter.Writer, itemCounter int, opts listOptions) error { - if itemCounter > 0 && !opts.long && itemCounter%4 != 0 { - _, _ = fmt.Fprintln(w) - } - return w.Flush() -} - // lsCmd represents the ls command var lsCmd = &cobra.Command{ Use: "ls [flags] []", diff --git a/cmd/ls_test.go b/cmd/ls_test.go index 5170a415..33528d83 100644 --- a/cmd/ls_test.go +++ b/cmd/ls_test.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" "testing" - "text/tabwriter" "time" "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox" @@ -21,13 +20,13 @@ func TestFormatFolderMetadata(t *testing.T) { } got := formatFolderMetadata(meta, false) - want := "/Documents\t" + want := "/Documents" if got != want { t.Errorf("formatFolderMetadata(short) = %q, want %q", got, want) } got = formatFolderMetadata(meta, true) - want = "-\t-\t-\t/Documents\t" + want = "-\t-\t-\t/Documents" if got != want { t.Errorf("formatFolderMetadata(long) = %q, want %q", got, want) } @@ -43,7 +42,7 @@ func TestFormatFileMetadata(t *testing.T) { } got := formatFileMetadata(meta, false) - want := "/test.txt\t" + want := "/test.txt" if got != want { t.Errorf("formatFileMetadata(short) = %q, want %q", got, want) } @@ -52,7 +51,7 @@ func TestFormatFileMetadata(t *testing.T) { if got == "" { t.Error("formatFileMetadata(long) returned empty string") } - if len(got) <= len("/test.txt\t") { + if len(got) <= len("/test.txt") { t.Errorf("formatFileMetadata(long) should include rev/size/time, got %q", got) } } @@ -65,13 +64,13 @@ func TestFormatDeletedMetadata(t *testing.T) { } got := formatDeletedMetadata(meta, false) - want := "/removed.txt\t" + want := "/removed.txt" if got != want { t.Errorf("formatDeletedMetadata(short) = %q, want %q", got, want) } got = formatDeletedMetadata(meta, true) - want = "-\t-\t-\t/removed.txt\t" + want = "-\t-\t-\t/removed.txt" if got != want { t.Errorf("formatDeletedMetadata(long) = %q, want %q", got, want) } @@ -137,21 +136,6 @@ func TestGetFileMetadataNotCalledForRoot(t *testing.T) { } } -func TestFinishListOutputAddsTrailingNewlineForPartialShortRows(t *testing.T) { - var out strings.Builder - w := new(tabwriter.Writer) - w.Init(&out, 4, 8, 1, ' ', 0) - - fmt.Fprint(w, "/one\t") - if err := finishListOutput(w, 1, listOptions{}); err != nil { - t.Fatalf("finishListOutput returned error: %v", err) - } - - if got := out.String(); !strings.HasSuffix(got, "\n") { - t.Fatalf("output %q does not end with newline", got) - } -} - func TestRenderLsResultsShortModeUsesFourColumns(t *testing.T) { entries := []files.IsMetadata{ &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/one"}}, diff --git a/cmd/root_test.go b/cmd/root_test.go index 895af92e..eda279be 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -207,6 +207,9 @@ func TestExecuteExitHelper(t *testing.T) { if os.Getenv("DBXCLI_TEST_EXECUTE_HELPER") != "1" { return } + if os.Getenv(listingOutputE2EEnv) == "1" { + installListingOutputFixture(t) + } separator := -1 for i, arg := range os.Args {