From 18bd28bda6c3e519589943823855e3eef9b857eb Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Wed, 18 Oct 2023 21:57:30 +1100 Subject: [PATCH] Show unstaged file names in default colour Previously, we had the following rules: * file names were in red when unstaged or partially staged * directory names were in red if unstaged, yellow if partially staged, and green if fully staged Red text on a black background can be hard to read, so instead I'm changing it so that unstaged and partially staged files have their names in the default text colour. So for files, the new rules are: * unstaged and partially staged files use the default colour * fully staged files are in green You can tell the difference between an unstaged and partially staged file by looking at the status characters (one of them will be in red so we do have some colour to indicate the difference). As for directories, the new rules are: * unstaged directories have their names in the default colour * partially staged directories have their folder arrows in green * fully staged directories have their folder arrows in green and their names in green I'm scrapping the yellow colour for partially staged directories because I always found it awkward and it never really clicked in my head as being halfway between green and red. I find having a little bit of green vs a lot of green a better indicator of the difference between partially staged and fully staged. I considered just leaving directories in the default colour regardless of their status, but that causes issues when a directory is collapsed. It's also nice to know if you've staged the last unstaged file in an directory. I've also applied the same pattern in the commit files view. Previously, partially staged files would also be yellow, and now they're the default colour but with a green status character (a half-filled circle). Fully staged files have a green name with a green full-circle status character. Also, if the default colour represents 'unstaged', but directories always have the default colour, then it's hard to know at a glance whether every file is staged. With the current rules (like before) if everything is staged, everything is green. I've also done a refactor on the code clean up some dead code from when the file tree outline was drawn with box characters, and I've made it so that the indentation in each line is handled inside the function that draws the line rather than in the recursive parent function. This makes it easier to experiment with things like showing the file status characters on the left edge of the view (admittedly after experimenting with it, I decided I didn't like it). Apologies for having a refactor and a functional change in the one commit but by the time I was done, I couldn't be bothered going back and retroactively splitting it into two halves. --- pkg/gui/context/commit_files_context.go | 2 +- pkg/gui/context/working_tree_context.go | 2 +- pkg/gui/presentation/files.go | 259 ++++++++++++++---------- 3 files changed, 158 insertions(+), 105 deletions(-) diff --git a/pkg/gui/context/commit_files_context.go b/pkg/gui/context/commit_files_context.go index 037554c91573..674e5d7d68c9 100644 --- a/pkg/gui/context/commit_files_context.go +++ b/pkg/gui/context/commit_files_context.go @@ -33,7 +33,7 @@ func NewCommitFilesContext(c *ContextCommon) *CommitFilesContext { return [][]string{{style.FgRed.Sprint("(none)")}} } - lines := presentation.RenderCommitFileTree(viewModel, c.Modes().Diffing.Ref, c.Git().Patch.PatchBuilder) + lines := presentation.RenderCommitFileTree(viewModel, c.Git().Patch.PatchBuilder) return lo.Map(lines, func(line string, _ int) []string { return []string{line} }) diff --git a/pkg/gui/context/working_tree_context.go b/pkg/gui/context/working_tree_context.go index 0e0b8d72b1ed..4be5f531fd79 100644 --- a/pkg/gui/context/working_tree_context.go +++ b/pkg/gui/context/working_tree_context.go @@ -24,7 +24,7 @@ func NewWorkingTreeContext(c *ContextCommon) *WorkingTreeContext { ) getDisplayStrings := func(_ int, _ int) [][]string { - lines := presentation.RenderFileTree(viewModel, c.Modes().Diffing.Ref, c.Model().Submodules) + lines := presentation.RenderFileTree(viewModel, c.Model().Submodules) return lo.Map(lines, func(line string, _ int) []string { return []string{line} }) diff --git a/pkg/gui/presentation/files.go b/pkg/gui/presentation/files.go index 1df90176f403..370c864ea0af 100644 --- a/pkg/gui/presentation/files.go +++ b/pkg/gui/presentation/files.go @@ -18,141 +18,140 @@ const ( COLLAPSED_ARROW = "▶" ) -// keeping these here as individual constants in case later on people want the old tree shape -const ( - INNER_ITEM = " " - LAST_ITEM = " " - NESTED = " " - NOTHING = " " -) - func RenderFileTree( tree filetree.IFileTree, - diffName string, submoduleConfigs []*models.SubmoduleConfig, ) []string { - return renderAux(tree.GetRoot().Raw(), tree.CollapsedPaths(), "", -1, func(node *filetree.Node[models.File], depth int) string { + collapsedPaths := tree.CollapsedPaths() + return renderAux(tree.GetRoot().Raw(), collapsedPaths, -1, -1, func(node *filetree.Node[models.File], treeDepth int, visualDepth int, isCollapsed bool) string { fileNode := filetree.NewFileNode(node) - return getFileLine(fileNode.GetHasUnstagedChanges(), fileNode.GetHasStagedChanges(), fileNameAtDepth(node, depth), diffName, submoduleConfigs, node.File) + return getFileLine(isCollapsed, fileNode.GetHasUnstagedChanges(), fileNode.GetHasStagedChanges(), treeDepth, visualDepth, submoduleConfigs, node) }) } func RenderCommitFileTree( tree *filetree.CommitFileTreeViewModel, - diffName string, patchBuilder *patch.PatchBuilder, ) []string { - return renderAux(tree.GetRoot().Raw(), tree.CollapsedPaths(), "", -1, func(node *filetree.Node[models.CommitFile], depth int) string { - // This is a little convoluted because we're dealing with either a leaf or a non-leaf. - // But this code actually applies to both. If it's a leaf, the status will just - // be whatever status it is, but if it's a non-leaf it will determine its status - // based on the leaves of that subtree - var status patch.PatchStatus - if node.EveryFile(func(file *models.CommitFile) bool { - return patchBuilder.GetFileStatus(file.Name, tree.GetRef().RefName()) == patch.WHOLE - }) { - status = patch.WHOLE - } else if node.EveryFile(func(file *models.CommitFile) bool { - return patchBuilder.GetFileStatus(file.Name, tree.GetRef().RefName()) == patch.UNSELECTED - }) { - status = patch.UNSELECTED - } else { - status = patch.PART - } + collapsedPaths := tree.CollapsedPaths() + return renderAux(tree.GetRoot().Raw(), collapsedPaths, -1, -1, func(node *filetree.Node[models.CommitFile], treeDepth int, visualDepth int, isCollapsed bool) string { + status := commitFilePatchStatus(node, tree, patchBuilder) - return getCommitFileLine(commitFileNameAtDepth(node, depth), diffName, node.File, status) + return getCommitFileLine(isCollapsed, treeDepth, visualDepth, node, status) }) } +// Returns the status of a commit file in terms of its inclusion in the custom patch +func commitFilePatchStatus(node *filetree.Node[models.CommitFile], tree *filetree.CommitFileTreeViewModel, patchBuilder *patch.PatchBuilder) patch.PatchStatus { + // This is a little convoluted because we're dealing with either a leaf or a non-leaf. + // But this code actually applies to both. If it's a leaf, the status will just + // be whatever status it is, but if it's a non-leaf it will determine its status + // based on the leaves of that subtree + if node.EveryFile(func(file *models.CommitFile) bool { + return patchBuilder.GetFileStatus(file.Name, tree.GetRef().RefName()) == patch.WHOLE + }) { + return patch.WHOLE + } else if node.EveryFile(func(file *models.CommitFile) bool { + return patchBuilder.GetFileStatus(file.Name, tree.GetRef().RefName()) == patch.UNSELECTED + }) { + return patch.UNSELECTED + } else { + return patch.PART + } +} + func renderAux[T any]( node *filetree.Node[T], collapsedPaths *filetree.CollapsedPaths, - prefix string, - depth int, - renderLine func(*filetree.Node[T], int) string, + // treeDepth is the depth of the node in the actual file tree. This is different to + // visualDepth because some directory nodes are compressed e.g. 'pkg/gui/blah' takes + // up two tree depths, but one visual depth. We need to track these separately, + // because indentation relies on visual depth, whereas file path truncation + // relies on tree depth. + treeDepth int, + visualDepth int, + renderLine func(*filetree.Node[T], int, int, bool) string, ) []string { if node == nil { return []string{} } - isRoot := depth == -1 + isRoot := treeDepth == -1 if node.IsFile() { if isRoot { return []string{} } - return []string{prefix + renderLine(node, depth)} - } - - if collapsedPaths.IsCollapsed(node.GetPath()) { - return []string{prefix + COLLAPSED_ARROW + " " + renderLine(node, depth)} + return []string{renderLine(node, treeDepth, visualDepth, false)} } arr := []string{} if !isRoot { - arr = append(arr, prefix+EXPANDED_ARROW+" "+renderLine(node, depth)) + isCollapsed := collapsedPaths.IsCollapsed(node.GetPath()) + arr = append(arr, renderLine(node, treeDepth, visualDepth, isCollapsed)) } - newPrefix := prefix - if strings.HasSuffix(prefix, LAST_ITEM) { - newPrefix = strings.TrimSuffix(prefix, LAST_ITEM) + NOTHING - } else if strings.HasSuffix(prefix, INNER_ITEM) { - newPrefix = strings.TrimSuffix(prefix, INNER_ITEM) + NESTED + if collapsedPaths.IsCollapsed(node.GetPath()) { + return arr } - for i, child := range node.Children { - isLast := i == len(node.Children)-1 - - var childPrefix string - if isRoot { - childPrefix = newPrefix - } else if isLast { - childPrefix = newPrefix + LAST_ITEM - } else { - childPrefix = newPrefix + INNER_ITEM - } - - arr = append(arr, renderAux(child, collapsedPaths, childPrefix, depth+1+node.CompressionLevel, renderLine)...) + for _, child := range node.Children { + arr = append(arr, renderAux(child, collapsedPaths, treeDepth+1+node.CompressionLevel, visualDepth+1, renderLine)...) } return arr } -func getFileLine(hasUnstagedChanges bool, hasStagedChanges bool, name string, diffName string, submoduleConfigs []*models.SubmoduleConfig, file *models.File) string { - // potentially inefficient to be instantiating these color - // objects with each render - partiallyModifiedColor := style.FgYellow - - restColor := style.FgGreen - if name == diffName { - restColor = theme.DiffTerminalColor - } else if file == nil && hasStagedChanges && hasUnstagedChanges { - restColor = partiallyModifiedColor - } else if hasUnstagedChanges { - restColor = theme.UnstagedChangesColor +// By default, folder names and file names are in the default text color. +// If a file is fully staged, its name will be green. +// If a directory is partially staged, its arrow will be green +// If a directory is fully staged, its name will be green +// Previously we used yellow for partially staged directories, but it never +// felt right. +// Also previously we had unstaged files in red, but I found that hard on the eyes. +func getFileLine( + isCollapsed bool, + hasUnstagedChanges bool, + hasStagedChanges bool, + treeDepth int, + visualDepth int, + submoduleConfigs []*models.SubmoduleConfig, + node *filetree.Node[models.File], +) string { + name := fileNameAtDepth(node, treeDepth) + output := "" + + restColor := theme.DefaultTextColor + nameColor := restColor + + file := node.File + + indentation := strings.Repeat(" ", visualDepth) + + isFullyStaged := hasStagedChanges && !hasUnstagedChanges + if isFullyStaged { + nameColor = style.FgGreen } - output := "" - if file != nil { - // this is just making things look nice when the background attribute is 'reverse' - firstChar := file.ShortStatus[0:1] - firstCharCl := style.FgGreen - if firstChar == "?" { - firstCharCl = theme.UnstagedChangesColor - } else if firstChar == " " { - firstCharCl = restColor + if file == nil { + output += indentation + "" + arrow := EXPANDED_ARROW + if isCollapsed { + arrow = COLLAPSED_ARROW } - secondChar := file.ShortStatus[1:2] - secondCharCl := theme.UnstagedChangesColor - if secondChar == " " { - secondCharCl = restColor + arrowStyle := theme.DefaultTextColor + if hasStagedChanges { + arrowStyle = style.FgGreen } - output = firstCharCl.Sprint(firstChar) - output += secondCharCl.Sprint(secondChar) - output += restColor.Sprint(" ") + output += arrowStyle.Sprint(arrow) + " " + } else { + // Sprinting the space at the end in the specific style is for the sake of + // when a reverse style is used in the theme, which looks ugly if you just + // use the default style + output += indentation + formatFileStatus(file, restColor) + restColor.Sprint(" ") } isSubmodule := file != nil && file.IsSubmodule(submoduleConfigs) @@ -162,10 +161,10 @@ func getFileLine(hasUnstagedChanges bool, hasStagedChanges bool, name string, di if icons.IsIconEnabled() { icon := icons.IconForFile(name, isSubmodule, isLinkedWorktree, isDirectory) paint := color.C256(icon.Color, false) - output += paint.Sprint(icon.Icon) + " " + output += paint.Sprint(icon.Icon) + restColor.Sprint(" ") } - output += restColor.Sprint(utils.EscapeSpecialChars(name)) + output += nameColor.Sprint(utils.EscapeSpecialChars(name)) if isSubmodule { output += theme.DefaultTextColor.Sprint(" (submodule)") @@ -174,31 +173,85 @@ func getFileLine(hasUnstagedChanges bool, hasStagedChanges bool, name string, di return output } -func getCommitFileLine(name string, diffName string, commitFile *models.CommitFile, status patch.PatchStatus) string { - var colour style.TextStyle - if diffName == name { - colour = theme.DiffTerminalColor - } else { +func formatFileStatus(file *models.File, restColor style.TextStyle) string { + firstChar := file.ShortStatus[0:1] + firstCharCl := style.FgGreen + if firstChar == "?" { + firstCharCl = theme.UnstagedChangesColor + } else if firstChar == " " { + firstCharCl = restColor + } + + secondChar := file.ShortStatus[1:2] + secondCharCl := theme.UnstagedChangesColor + if secondChar == " " { + secondCharCl = restColor + } + + return firstCharCl.Sprint(firstChar) + secondCharCl.Sprint(secondChar) +} + +func getCommitFileLine( + isCollapsed bool, + treeDepth int, + visualDepth int, + node *filetree.Node[models.CommitFile], + status patch.PatchStatus, +) string { + indentation := strings.Repeat(" ", visualDepth) + name := commitFileNameAtDepth(node, treeDepth) + commitFile := node.File + output := indentation + + isDirectory := commitFile == nil + + nameColor := theme.DefaultTextColor + + if isDirectory { + arrow := EXPANDED_ARROW + if isCollapsed { + arrow = COLLAPSED_ARROW + } + + var arrowColor style.TextStyle switch status { case patch.WHOLE: - colour = style.FgGreen + arrowColor = style.FgGreen + nameColor = style.FgGreen case patch.PART: - colour = style.FgYellow + arrowColor = style.FgGreen + nameColor = theme.DefaultTextColor case patch.UNSELECTED: - colour = theme.DefaultTextColor + arrowColor = theme.DefaultTextColor + nameColor = theme.DefaultTextColor } - } - output := "" + output += arrowColor.Sprint(arrow) + " " + } else { + var symbol string + var symbolStyle style.TextStyle - name = utils.EscapeSpecialChars(name) - if commitFile != nil { - output += getColorForChangeStatus(commitFile.ChangeStatus).Sprint(commitFile.ChangeStatus) + " " + switch status { + case patch.WHOLE: + nameColor = style.FgGreen + symbol = "●" + symbolStyle = style.FgGreen + case patch.PART: + nameColor = theme.DefaultTextColor + symbol = "◐" + symbolStyle = style.FgGreen + case patch.UNSELECTED: + nameColor = theme.DefaultTextColor + symbol = commitFile.ChangeStatus + symbolStyle = getColorForChangeStatus(commitFile.ChangeStatus) + } + + output += symbolStyle.Sprint(symbol) + " " } + name = utils.EscapeSpecialChars(name) isSubmodule := false isLinkedWorktree := false - isDirectory := commitFile == nil if icons.IsIconEnabled() { icon := icons.IconForFile(name, isSubmodule, isLinkedWorktree, isDirectory) @@ -206,7 +259,7 @@ func getCommitFileLine(name string, diffName string, commitFile *models.CommitFi output += paint.Sprint(icon.Icon) + " " } - output += colour.Sprint(name) + output += nameColor.Sprint(name) return output }