diff --git a/pkg/gui/context/reflog_commits_context.go b/pkg/gui/context/reflog_commits_context.go index 6358fbbb0..71314bf66 100644 --- a/pkg/gui/context/reflog_commits_context.go +++ b/pkg/gui/context/reflog_commits_context.go @@ -27,13 +27,10 @@ func NewReflogCommitsContext(c *ContextCommon) *ReflogCommitsContext { ) getDisplayStrings := func(startIdx int, endIdx int) [][]string { - commits := viewModel.GetItems() - if startIdx >= len(commits) { - return nil - } - return presentation.GetReflogCommitListDisplayStrings( - commits[startIdx:endIdx], + viewModel.GetItems(), + startIdx, + endIdx, c.State().GetRepoState().GetScreenMode() != types.SCREEN_NORMAL, c.Modes().CherryPicking.SelectedHashSet(), c.Modes().Diffing.Ref, diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 67fa62ac8..ace3b1b3c 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -177,6 +177,10 @@ func GetCommitListDisplayStrings( (hasRebaseUpdateRefsConfig || b.CommitHash != commits[0].Hash()) })) + reservedWidths := getReservedColumnWidths( + commits, common.UserConfig().Gui.CommitHashLength, fullDescription, + timeFormat, shortTimeFormat, now, bisectInfo, bisectBounds) + lines := make([][]string, 0, len(filteredCommits)) var bisectStatus BisectStatus willBeRebased := markedBaseCommit == "" @@ -204,11 +208,64 @@ func GetCommitListDisplayStrings( fullDescription, bisectStatus, bisectInfo, + reservedWidths, )) } return lines } +// The width of a column is the width of the widest string in it, and a column +// whose strings are all empty is left out entirely. The panels that show a +// commit list hand over only the lines that are on screen, so several columns +// would change their width, or come and go, as the user scrolls. These are the +// widths those columns need for all the commits in the list. +type reservedColumnWidths struct { + hash int + bisect int + description int + action int +} + +// precondition: commits is not empty +func getReservedColumnWidths( + commits []*models.Commit, + hashLength int, + fullDescription bool, + timeFormat string, + shortTimeFormat string, + now time.Time, + bisectInfo *git_commands.BisectInfo, + bisectBounds *bisectBounds, +) reservedColumnWidths { + result := reservedColumnWidths{} + + for i, commit := range commits { + result.hash = max(result.hash, utils.StringWidth(getHashText(commit, hashLength))) + if commit.IsTODO() { + result.action = max(result.action, utils.StringWidth(getActionText(commit))) + } + bisectStatus := getBisectStatus(i, commit.Hash(), bisectInfo, bisectBounds) + result.bisect = max(result.bisect, + utils.StringWidth(getBisectStatusText(bisectStatus, bisectInfo))) + } + + if fullDescription { + // Formatting the date of every commit on every render would be too + // expensive, so measure the oldest one only. It is the one least likely + // to be from today, and so the one most likely to be shown in the long + // time format; with a conventional time format that one is both wider + // than the short format and the same width for every date, which makes + // it the width the whole column needs. An unconventional format can + // break either of those assumptions, and then some of the column's + // width still comes and goes; it can never reserve more width than one + // of the commits asks for, though. + result.description = utils.StringWidth(utils.UnixToDateSmart( + now, commits[len(commits)-1].UnixTimestamp, timeFormat, shortTimeFormat)) + } + + return result +} + func getbisectBounds(commits []*models.Commit, bisectInfo *git_commands.BisectInfo) *bisectBounds { if !bisectInfo.Bisecting() { return nil @@ -313,24 +370,18 @@ func getBisectStatus(index int, commitHash string, bisectInfo *git_commands.Bise } func getBisectStatusText(bisectStatus BisectStatus, bisectInfo *git_commands.BisectInfo) string { - if bisectStatus == BisectStatusNone { - return "" - } - - style := getBisectStatusColor(bisectStatus) - switch bisectStatus { case BisectStatusNew: - return style.Sprintf("<-- " + bisectInfo.NewTerm()) + return "<-- " + bisectInfo.NewTerm() case BisectStatusOld: - return style.Sprintf("<-- " + bisectInfo.OldTerm()) + return "<-- " + bisectInfo.OldTerm() case BisectStatusCurrent: // TODO: i18n - return style.Sprintf("<-- current") + return "<-- current" case BisectStatusSkipped: - return style.Sprintf("<-- skipped") + return "<-- skipped" case BisectStatusCandidate: - return style.Sprintf("?") + return "?" case BisectStatusNone: return "" } @@ -338,6 +389,33 @@ func getBisectStatusText(bisectStatus BisectStatus, bisectInfo *git_commands.Bis return "" } +func getHashText(commit *models.Commit, hashLength int) string { + hash := commit.Hash() + if hashLength >= len(hash) { + return hash + } + if hashLength > 0 { + return hash[:hashLength] + } + if !icons.IsIconEnabled() { // hashLength <= 0 + return "*" + } + return "" +} + +func getActionText(commit *models.Commit) string { + if commit.Action == models.ActionNone { + return "" + } + + text := commit.Action.String() + // Only show the flag for fixup commands (where -C changes the meaning) + if commit.ActionFlag != "" && commit.Action == todo.Fixup { + text += " " + commit.ActionFlag + } + return text +} + func displayCommit( common *common.Common, commit *models.Commit, @@ -355,18 +433,17 @@ func displayCommit( fullDescription bool, bisectStatus BisectStatus, bisectInfo *git_commands.BisectInfo, + reservedWidths reservedColumnWidths, ) []string { - bisectString := getBisectStatusText(bisectStatus, bisectInfo) + bisectString := "" + if bisectText := getBisectStatusText(bisectStatus, bisectInfo); bisectText != "" { + bisectString = getBisectStatusColor(bisectStatus).Sprint(bisectText) + } - hashString := "" hashColor := getHashColor(commit, diffName, cherryPickedCommitHashSet, bisectStatus, bisectInfo) - hashLength := common.UserConfig().Gui.CommitHashLength - if hashLength >= len(commit.Hash()) { - hashString = hashColor.Sprint(commit.Hash()) - } else if hashLength > 0 { - hashString = hashColor.Sprint(commit.Hash()[:hashLength]) - } else if !icons.IsIconEnabled() { // hashLength <= 0 - hashString = hashColor.Sprint("*") + hashString := "" + if hashText := getHashText(commit, common.UserConfig().Gui.CommitHashLength); hashText != "" { + hashString = hashColor.Sprint(hashText) } divergenceString := "" @@ -384,13 +461,8 @@ func displayCommit( } actionString := "" - if commit.Action != models.ActionNone { - actionStr := commit.Action.String() - // Only show the flag for fixup commands (where -C changes the meaning) - if commit.ActionFlag != "" && commit.Action == todo.Fixup { - actionStr += " " + commit.ActionFlag - } - actionString = actionColorMap(commit.Action, commit.Status).Sprint(actionStr) + if actionText := getActionText(commit); actionText != "" { + actionString = actionColorMap(commit.Action, commit.Status).Sprint(actionText) } tagString := "" @@ -443,10 +515,10 @@ func displayCommit( cols = append( cols, divergenceString, - hashString, - bisectString, - descriptionString, - actionString, + utils.WithPadding(hashString, reservedWidths.hash, utils.AlignLeft), + utils.WithPadding(bisectString, reservedWidths.bisect, utils.AlignLeft), + utils.WithPadding(descriptionString, reservedWidths.description, utils.AlignLeft), + utils.WithPadding(actionString, reservedWidths.action, utils.AlignLeft), author, graphLine+mark+tagString+theme.DefaultTextColor.Sprint(name), ) diff --git a/pkg/gui/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go index 12e0fc1d6..3c3ce0044 100644 --- a/pkg/gui/presentation/commits_test.go +++ b/pkg/gui/presentation/commits_test.go @@ -17,8 +17,13 @@ import ( "github.com/xo/terminfo" ) +// Scenarios write their expected output as a raw string literal, indented with +// tabs so that it lines up with the surrounding code. Strip that indentation, +// along with the newlines after the opening backtick and before the closing +// one. Spaces are left alone, so that a scenario can expect a line that starts +// with an empty column. func formatExpected(expected string) string { - return strings.TrimSpace(strings.ReplaceAll(expected, "\t", "")) + return strings.Trim(strings.ReplaceAll(expected, "\t", ""), "\n") } func TestGetCommitListDisplayStrings(t *testing.T) { @@ -270,8 +275,8 @@ func TestGetCommitListDisplayStrings(t *testing.T) { cherryPickedCommitHashSet: set.New[string](), now: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), expected: formatExpected(` - hash4 ○ commit4 - hash5 ○ commit5 + hash4 ○ commit4 + hash5 ○ commit5 `), }, { @@ -333,6 +338,25 @@ func TestGetCommitListDisplayStrings(t *testing.T) { hash2 pick commit2 `), }, + { + testName: "only showing TODO commits that have no hash", + commitOpts: []models.NewCommitOpts{ + {Name: "refs/heads/branch1", Action: todo.UpdateRef}, + {Name: "refs/heads/branch2", Action: todo.UpdateRef}, + {Name: "commit1", Hash: "hash1", Parents: []string{"hash2"}, Action: todo.Pick}, + {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}}, + }, + startIdx: 0, + endIdx: 2, + showGraph: false, + bisectInfo: git_commands.NewNullBisectInfo(), + cherryPickedCommitHashSet: set.New[string](), + now: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), + expected: formatExpected(` + update-ref branch1 + update-ref branch2 + `), + }, { testName: "graph in divergence view - all commits visible", commitOpts: []models.NewCommitOpts{ @@ -525,6 +549,25 @@ func TestGetCommitListDisplayStrings(t *testing.T) { hash2 2019-12-20 Jesse Duffield commit2 `), }, + { + testName: "only showing commits from today", + commitOpts: []models.NewCommitOpts{ + {Name: "commit1", Hash: "hash1", UnixTimestamp: 1577844184, AuthorName: "Jesse Duffield"}, + {Name: "commit2", Hash: "hash2", UnixTimestamp: 1576844184, AuthorName: "Jesse Duffield"}, + }, + fullDescription: true, + timeFormat: "2006-01-02", + shortTimeFormat: "3:04PM", + startIdx: 0, + endIdx: 1, + showGraph: false, + bisectInfo: git_commands.NewNullBisectInfo(), + cherryPickedCommitHashSet: set.New[string](), + now: time.Date(2020, 1, 1, 5, 3, 4, 0, time.UTC), + expected: formatExpected(` + hash1 2:03AM Jesse Duffield commit1 + `), + }, } oldColorLevel := color.ForceSetColorLevel(terminfo.ColorLevelNone) diff --git a/pkg/gui/presentation/reflog_commits.go b/pkg/gui/presentation/reflog_commits.go index b84a10e59..50305b1d3 100644 --- a/pkg/gui/presentation/reflog_commits.go +++ b/pkg/gui/presentation/reflog_commits.go @@ -12,25 +12,35 @@ import ( "github.com/samber/lo" ) -func GetReflogCommitListDisplayStrings(commits []*models.Commit, fullDescription bool, cherryPickedCommitHashSet *set.Set[string], diffName string, now time.Time, timeFormat string, shortTimeFormat string, parseEmoji bool) [][]string { +func GetReflogCommitListDisplayStrings(commits []*models.Commit, startIdx int, endIdx int, fullDescription bool, cherryPickedCommitHashSet *set.Set[string], diffName string, now time.Time, timeFormat string, shortTimeFormat string, parseEmoji bool) [][]string { + if startIdx >= len(commits) { + return nil + } + var displayFunc func(*models.Commit, reflogCommitDisplayAttributes) []string + reservedDateWidth := 0 if fullDescription { displayFunc = getFullDescriptionDisplayStringsForReflogCommit + // See getReservedColumnWidths for why the oldest entry alone decides + // how much width the date column needs. + reservedDateWidth = utils.StringWidth(utils.UnixToDateSmart( + now, commits[len(commits)-1].UnixTimestamp, timeFormat, shortTimeFormat)) } else { displayFunc = getDisplayStringsForReflogCommit } - return lo.Map(commits, func(commit *models.Commit, _ int) []string { + return lo.Map(commits[startIdx:endIdx], func(commit *models.Commit, _ int) []string { diffed := commit.Hash() == diffName cherryPicked := cherryPickedCommitHashSet.Includes(commit.Hash()) return displayFunc(commit, reflogCommitDisplayAttributes{ - cherryPicked: cherryPicked, - diffed: diffed, - parseEmoji: parseEmoji, - timeFormat: timeFormat, - shortTimeFormat: shortTimeFormat, - now: now, + cherryPicked: cherryPicked, + diffed: diffed, + parseEmoji: parseEmoji, + timeFormat: timeFormat, + shortTimeFormat: shortTimeFormat, + now: now, + reservedDateWidth: reservedDateWidth, }) }) } @@ -55,6 +65,9 @@ type reflogCommitDisplayAttributes struct { timeFormat string shortTimeFormat string now time.Time + // The width the date column needs for the whole reflog, not just for the + // lines that are on screen + reservedDateWidth int } func getFullDescriptionDisplayStringsForReflogCommit(c *models.Commit, attrs reflogCommitDisplayAttributes) []string { @@ -63,9 +76,12 @@ func getFullDescriptionDisplayStringsForReflogCommit(c *models.Commit, attrs ref name = emoji.Sprint(name) } + date := style.FgMagenta.Sprint( + utils.UnixToDateSmart(attrs.now, c.UnixTimestamp, attrs.timeFormat, attrs.shortTimeFormat)) + return []string{ reflogHashColor(attrs.cherryPicked, attrs.diffed).Sprint(c.ShortHash()), - style.FgMagenta.Sprint(utils.UnixToDateSmart(attrs.now, c.UnixTimestamp, attrs.timeFormat, attrs.shortTimeFormat)), + utils.WithPadding(date, attrs.reservedDateWidth, utils.AlignLeft), theme.DefaultTextColor.Sprint(name), } } diff --git a/pkg/gui/presentation/reflog_commits_test.go b/pkg/gui/presentation/reflog_commits_test.go new file mode 100644 index 000000000..9a81a7021 --- /dev/null +++ b/pkg/gui/presentation/reflog_commits_test.go @@ -0,0 +1,116 @@ +package presentation + +import ( + "strings" + "testing" + "time" + + "github.com/gookit/color" + "github.com/jesseduffield/generics/set" + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" + "github.com/xo/terminfo" +) + +func TestGetReflogCommitListDisplayStrings(t *testing.T) { + scenarios := []struct { + testName string + commitOpts []models.NewCommitOpts + fullDescription bool + timeFormat string + shortTimeFormat string + now time.Time + startIdx int + endIdx int + expected string + }{ + { + testName: "no commits", + commitOpts: []models.NewCommitOpts{}, + startIdx: 0, + endIdx: 1, + now: time.Date(2020, 1, 1, 5, 3, 4, 0, time.UTC), + expected: "", + }, + { + testName: "some commits", + commitOpts: []models.NewCommitOpts{ + {Name: "checkout: moving from master to mybranch", Hash: "hash1"}, + {Name: "commit: make a change", Hash: "hash2"}, + }, + startIdx: 0, + endIdx: 2, + now: time.Date(2020, 1, 1, 5, 3, 4, 0, time.UTC), + expected: formatExpected(` + hash1 checkout: moving from master to mybranch + hash2 commit: make a change + `), + }, + { + testName: "full description", + commitOpts: []models.NewCommitOpts{ + {Name: "commit: today", Hash: "hash1", UnixTimestamp: 1577844184}, + {Name: "commit: a while ago", Hash: "hash2", UnixTimestamp: 1576844184}, + }, + fullDescription: true, + timeFormat: "2006-01-02", + shortTimeFormat: "3:04PM", + startIdx: 0, + endIdx: 2, + now: time.Date(2020, 1, 1, 5, 3, 4, 0, time.UTC), + expected: formatExpected(` + hash1 2:03AM commit: today + hash2 2019-12-20 commit: a while ago + `), + }, + { + testName: "only showing commits from today", + commitOpts: []models.NewCommitOpts{ + {Name: "commit: today", Hash: "hash1", UnixTimestamp: 1577844184}, + {Name: "commit: a while ago", Hash: "hash2", UnixTimestamp: 1576844184}, + }, + fullDescription: true, + timeFormat: "2006-01-02", + shortTimeFormat: "3:04PM", + startIdx: 0, + endIdx: 1, + now: time.Date(2020, 1, 1, 5, 3, 4, 0, time.UTC), + expected: formatExpected(` + hash1 2:03AM commit: today + `), + }, + } + + oldColorLevel := color.ForceSetColorLevel(terminfo.ColorLevelNone) + defer color.ForceSetColorLevel(oldColorLevel) + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + hashPool := &utils.StringPool{} + + commits := lo.Map(s.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + + result := GetReflogCommitListDisplayStrings( + commits, + s.startIdx, + s.endIdx, + s.fullDescription, + set.New[string](), + "", + s.now, + s.timeFormat, + s.shortTimeFormat, + false, + ) + + renderedLines, _ := utils.RenderDisplayStrings(result, nil) + renderedResult := strings.Join(renderedLines, "\n") + t.Logf("\n%s", renderedResult) + + assert.EqualValues(t, s.expected, renderedResult) + }) + } +} diff --git a/pkg/utils/color.go b/pkg/utils/color.go index 17e400337..7dfeec093 100644 --- a/pkg/utils/color.go +++ b/pkg/utils/color.go @@ -12,21 +12,22 @@ import ( var ( decoloriseCache = make(map[string]string) decoloriseMutex sync.RWMutex + + colorCodeRe = regexp.MustCompile(`\x1B\[([0-9]{1,3}(;[0-9]{1,3})*)?[mGK]`) + linkRe = regexp.MustCompile(`\x1B]8;[^;]*;(.*?)(\x1B.|\x07)`) ) // Decolorise strips a string of color func Decolorise(str string) string { decoloriseMutex.RLock() - val := decoloriseCache[str] + val, ok := decoloriseCache[str] decoloriseMutex.RUnlock() - if val != "" { + if ok { return val } - re := regexp.MustCompile(`\x1B\[([0-9]{1,3}(;[0-9]{1,3})*)?[mGK]`) - linkRe := regexp.MustCompile(`\x1B]8;[^;]*;(.*?)(\x1B.|\x07)`) - ret := re.ReplaceAllString(str, "") + ret := colorCodeRe.ReplaceAllString(str, "") ret = linkRe.ReplaceAllString(ret, "") decoloriseMutex.Lock() diff --git a/pkg/utils/formatting.go b/pkg/utils/formatting.go index 080f58f87..4fb1702f4 100644 --- a/pkg/utils/formatting.go +++ b/pkg/utils/formatting.go @@ -36,6 +36,12 @@ func StringWidth(s string) int { // WithPadding pads a string as much as you want func WithPadding(str string, padding int, alignment Alignment) string { + if padding <= 0 { + // Nothing to pad to, and measuring the string isn't free: Decolorise + // compiles a regex whenever it is called with a string it hasn't cached. + return str + } + uncoloredStr := Decolorise(str) width := StringWidth(uncoloredStr) if padding < width {