diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 3821c5768..228affe31 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -103,22 +103,86 @@ func (self *ReposHelper) getCurrentBranch(path string) string { return self.c.Tr.BranchUnknown } -// The maximum width of the repo name and branch name columns of the recent -// repos menu. Without a limit, one long name pushes the path column off the -// right edge of the menu for every entry, because each column is padded to the -// width of its widest entry. -const recentReposColumnMaxWidth = 30 +// The most that the name and the branch column of the recent repos menu are +// allowed to take up. Each column is padded to the width of its widest entry, +// so without a limit one long entry pushes the columns after it off the right +// edge of the menu for every entry. +const ( + recentReposNameMaxWidth = 30 + recentReposBranchMaxWidth = 30 +) -func (self *ReposHelper) recentRepoMenuItem(path string, branchName string) *types.MenuItem { - repoName := filepath.Base(path) - displayedRepoName := utils.TruncateWithEllipsis(repoName, recentReposColumnMaxWidth) +// One entry of the recent repos menu. +type recentRepoEntry struct { + // What the entry stands for + path string + branchName string - // The icon is part of the column, so it counts towards the maximum width. + // The text of its three columns + nameColumn string + branchColumn string + dirColumn string +} + +func newRecentRepoEntry(path string, branchName string) recentRepoEntry { + // The icon is part of the column, so it counts towards the column's width. branchColumn := branchName if icons.IsIconEnabled() { branchColumn = icons.BRANCH_ICON + " " + branchName } - displayedBranchColumn := utils.TruncateWithEllipsis(branchColumn, recentReposColumnMaxWidth) + + return recentRepoEntry{ + path: path, + branchName: branchName, + nameColumn: filepath.Base(path), + branchColumn: branchColumn, + // The last segment of the path is already in the first column, so the + // directory that contains the repo is enough to tell repos with the + // same name apart. + dirColumn: utils.ContractTilde(filepath.Dir(path)), + } +} + +// How wide the columns of the recent repos menu are allowed to get. +type recentRepoColumnWidths struct { + name int + branch int + dir int +} + +// Gives the name and the branch column as much as their entries need, up to +// their respective maximum, and the rest of the row to the directory column. +// Measuring the entries first matters because most users don't have names that +// long; truncating the directories as if they did would cut them short for no +// reason. +func (self *ReposHelper) fitRecentRepoColumns(entries []recentRepoEntry) recentRepoColumnWidths { + // The menu appends a Cancel entry, whose label sits in the first column. + nameWidth := utils.StringWidth(self.c.Tr.Cancel) + branchWidth := 0 + for _, entry := range entries { + nameWidth = max(nameWidth, utils.StringWidth(entry.nameColumn)) + branchWidth = max(branchWidth, utils.StringWidth(entry.branchColumn)) + } + + nameWidth = min(nameWidth, recentReposNameMaxWidth) + branchWidth = min(branchWidth, recentReposBranchMaxWidth) + + return recentRepoColumnWidths{ + name: nameWidth, + branch: branchWidth, + // The menu's frame takes up two columns, and two more separate the + // three columns from each other. What's left is the room a whole row + // has in a menu that is as wide as it gets. + dir: menuMaxWidth - 2 - 2 - nameWidth - branchWidth, + } +} + +func (self *ReposHelper) recentRepoMenuItem(entry recentRepoEntry, widths recentRepoColumnWidths) *types.MenuItem { + displayedName := utils.TruncateWithEllipsis(entry.nameColumn, widths.name) + displayedBranch := utils.TruncateWithEllipsis(entry.branchColumn, widths.branch) + // The beginning and the end of a directory are both worth seeing, so it + // loses its middle rather than its end when it doesn't fit. + displayedDir := utils.TruncateWithEllipsisInMiddle(entry.dirColumn, widths.dir) // Spell out whatever the columns show in truncated form type tooltipField struct { @@ -130,11 +194,14 @@ func (self *ReposHelper) recentRepoMenuItem(path string, branchName string) *typ fields = append(fields, tooltipField{label: label, value: value}) } - if displayedRepoName != repoName { - addTooltipField(self.c.Tr.RecentReposRepoLabel, repoName) + if displayedName != entry.nameColumn { + addTooltipField(self.c.Tr.RecentReposRepoLabel, entry.nameColumn) } - if displayedBranchColumn != branchColumn { - addTooltipField(self.c.Tr.RecentReposBranchLabel, branchName) + if displayedBranch != entry.branchColumn { + addTooltipField(self.c.Tr.RecentReposBranchLabel, entry.branchName) + } + if displayedDir != entry.dirColumn { + addTooltipField(self.c.Tr.RecentReposPathLabel, entry.dirColumn) } // Line the values up behind the widest of the labels that are there @@ -147,16 +214,13 @@ func (self *ReposHelper) recentRepoMenuItem(path string, branchName string) *typ return &types.MenuItem{ LabelColumns: []string{ - displayedRepoName, - style.FgCyan.Sprint(displayedBranchColumn), - // The last segment of the path is already in the first column, so - // showing the directory that contains the repo is enough to tell - // repos with the same name apart. - style.FgMagenta.Sprint(utils.ContractTilde(filepath.Dir(path))), + displayedName, + style.FgCyan.Sprint(displayedBranch), + style.FgMagenta.Sprint(displayedDir), }, // Filtering matches the full text, including the parts that the columns // above truncate or leave out. - FilterColumns: []string{repoName, branchName, path}, + FilterColumns: []string{entry.nameColumn, entry.branchName, entry.path}, Tooltip: strings.Join(tooltipLines, "\n"), OnPress: func() error { // Check before clearing the stack, so a refused switch doesn't @@ -168,7 +232,7 @@ func (self *ReposHelper) recentRepoMenuItem(path string, branchName string) *typ // if we were in a submodule, we want to forget about that stack of repos // so that hitting escape in the new repo does nothing self.c.State().GetRepoPathStack().Clear() - return self.switchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) + return self.switchTo(entry.path, self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) }, } } @@ -195,8 +259,13 @@ func (self *ReposHelper) CreateRecentReposMenu() error { wg.Wait() - menuItems := lo.Map(recentRepoPaths, func(path string, i int) *types.MenuItem { - return self.recentRepoMenuItem(path, currentBranches[i]) + entries := lo.Map(recentRepoPaths, func(path string, i int) recentRepoEntry { + return newRecentRepoEntry(path, currentBranches[i]) + }) + + columnWidths := self.fitRecentRepoColumns(entries) + menuItems := lo.Map(entries, func(entry recentRepoEntry, _ int) *types.MenuItem { + return self.recentRepoMenuItem(entry, columnWidths) }) return self.c.Menu(types.CreateMenuOptions{ diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 918247902..0f33d9bc0 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -334,6 +334,7 @@ type TranslationSet struct { RecentRepos string RecentReposRepoLabel string RecentReposBranchLabel string + RecentReposPathLabel string MergeOptionsTitle string RebaseOptionsTitle string CherryPickOptionsTitle string @@ -1494,6 +1495,7 @@ func EnglishTranslationSet() *TranslationSet { RecentRepos: "Recent repositories", RecentReposRepoLabel: "Repo:", RecentReposBranchLabel: "Branch:", + RecentReposPathLabel: "Path:", MergeOptionsTitle: "Merge options", RebaseOptionsTitle: "Rebase options", CherryPickOptionsTitle: "Cherry-pick options", diff --git a/pkg/integration/tests/misc/recent_repos_column_widths.go b/pkg/integration/tests/misc/recent_repos_column_widths.go new file mode 100644 index 000000000..587e5d9b6 --- /dev/null +++ b/pkg/integration/tests/misc/recent_repos_column_widths.go @@ -0,0 +1,45 @@ +package misc + +import ( + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RecentReposColumnWidths = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The directory column of the recent repositories menu gets the room that the short columns before it don't need", + ExtraCmdArgs: []string{}, + ExtraEnvVars: map[string]string{ + "SHOW_RECENT_REPOS": "true", + }, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + // the first entry is the repo we're in, so it isn't offered + current, _ := filepath.Abs(".") + target, _ := filepath.Abs("../other") + cfg.GetAppState().RecentRepos = []string{current, target} + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("one") + shell.CloneNonBare("other") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // The name and the branch are short, so the directory is shown in full + // even though it is longer than the third of the row it would get if + // they each took their maximum width + t.ExpectPopup().Menu(). + Title(Equals("Recent repositories")). + Lines( + Contains("other master ~/_results/misc/recent_repos_column_widths/actual").IsSelected(), + Contains("Cancel"), + ). + Tap(func() { + // Nothing is truncated, so there is nothing to spell out + t.Views().Tooltip().IsInvisible() + }). + Confirm() + + t.Views().Status().Content(Contains("other → master")) + }, +}) diff --git a/pkg/integration/tests/misc/recent_repos_with_long_names.go b/pkg/integration/tests/misc/recent_repos_with_long_names.go index 7396faff3..7bb18593c 100644 --- a/pkg/integration/tests/misc/recent_repos_with_long_names.go +++ b/pkg/integration/tests/misc/recent_repos_with_long_names.go @@ -18,10 +18,14 @@ var RecentReposWithLongNames = NewIntegrationTest(NewIntegrationTestArgs{ // the first entry is the repo we're in, so it isn't offered current, _ := filepath.Abs(".") target, _ := filepath.Abs("../repo-with-a-name-that-is-far-too-long") - cfg.GetAppState().RecentRepos = []string{current, target} + other, _ := filepath.Abs("../other") + cfg.GetAppState().RecentRepos = []string{current, target, other} }, SetupRepo: func(shell *Shell) { shell.EmptyCommit("one") + // this one is cloned while master is checked out, so only its path is + // long enough to be truncated + shell.CloneNonBare("other") shell.NewBranch("branch-with-a-name-that-is-too-long") shell.CloneNonBare("repo-with-a-name-that-is-far-too-long") }, @@ -29,10 +33,20 @@ var RecentReposWithLongNames = NewIntegrationTest(NewIntegrationTestArgs{ t.ExpectPopup().Menu(). Title(Equals("Recent repositories")). Lines( - Contains("repo-with-a-name-that-is-far-… branch-with-a-name-that-is-to…").IsSelected(), + // The repos live in the test's own directory, so their paths are + // long enough to lose their middle + Contains("repo-with-a-name-that-is-far-… branch-with-a-name-that-is-to… ~/_results/mi…names/actual").IsSelected(), + Contains("other master ~/_results/mi…names/actual"), Contains("Cancel"), ). - Tooltip(Equals("Repo: repo-with-a-name-that-is-far-too-long\nBranch: branch-with-a-name-that-is-too-long")). + Tooltip(Contains("Repo: repo-with-a-name-that-is-far-too-long\n" + + "Branch: branch-with-a-name-that-is-too-long\n" + + "Path: ~/_results/misc/recent_repos_with_long_names/actual")). + // The values are lined up behind the labels that are there, so a + // tooltip that only spells out the path doesn't indent it as if a + // "Branch:" label were in front of it + Select(Contains("other")). + Tooltip(Equals("Path: ~/_results/misc/recent_repos_with_long_names/actual")). // Filtering matches the full names, including the part that the // menu truncates Filter("too-long"). diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6dcb226f1..c2f536bfa 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -365,6 +365,7 @@ var tests = []*components.IntegrationTest{ misc.DirenvUnloadsOnBlockedEnvrc, misc.FilterRecentRepos, misc.InitialOpen, + misc.RecentReposColumnWidths, misc.RecentReposOnLaunch, misc.RecentReposWithLongNames, misc.StartInGitDir,