Show file names in default colour (#3081)

Fixes https://github.com/jesseduffield/lazygit/issues/3077

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 files have their names in the default text colour.
I'm also making it so that partially staged files are in yellow, just
like how
partially staged directories are yellow (same deal with the commit files
view
when adding to a custom patch).

So the new rules are:
* unstaged files/directories use the default colour
* partially staged files/directories are in yellow
* fully staged files/directories are in 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

@@ -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}
 		})

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}
 		})

pkg/gui/presentation/files.go

@@ -18,141 +18,132 @@ 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 {
+	collapsedPaths := tree.CollapsedPaths()
-		// This is a little convoluted because we're dealing with either a leaf or a non-leaf.
+	return renderAux(tree.GetRoot().Raw(), collapsedPaths, -1, -1, func(node *filetree.Node[models.CommitFile], treeDepth int, visualDepth int, isCollapsed bool) string {
-		// But this code actually applies to both. If it's a leaf, the status will just
+		status := commitFilePatchStatus(node, tree, patchBuilder)
-		// 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
-		}
 
-		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,
+	// treeDepth is the depth of the node in the actual file tree. This is different to
-	depth int,
+	// visualDepth because some directory nodes are compressed e.g. 'pkg/gui/blah' takes
-	renderLine func(*filetree.Node[T], int) string,
+	// 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)}
+		return []string{renderLine(node, treeDepth, visualDepth, false)}
-	}
-
-	if collapsedPaths.IsCollapsed(node.GetPath()) {
-		return []string{prefix + COLLAPSED_ARROW + " " + renderLine(node, depth)}
 	}
 
 	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 collapsedPaths.IsCollapsed(node.GetPath()) {
-	if strings.HasSuffix(prefix, LAST_ITEM) {
+		return arr
-		newPrefix = strings.TrimSuffix(prefix, LAST_ITEM) + NOTHING
-	} else if strings.HasSuffix(prefix, INNER_ITEM) {
-		newPrefix = strings.TrimSuffix(prefix, INNER_ITEM) + NESTED
 	}
 
-	for i, child := range node.Children {
+	for _, child := range node.Children {
-		isLast := i == len(node.Children)-1
+		arr = append(arr, renderAux(child, collapsedPaths, treeDepth+1+node.CompressionLevel, visualDepth+1, renderLine)...)
-
-		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)...)
 	}
 
 	return arr
 }
 
-func getFileLine(hasUnstagedChanges bool, hasStagedChanges bool, name string, diffName string, submoduleConfigs []*models.SubmoduleConfig, file *models.File) string {
+func getFileLine(
-	// potentially inefficient to be instantiating these color
+	isCollapsed bool,
-	// objects with each render
+	hasUnstagedChanges bool,
-	partiallyModifiedColor := style.FgYellow
+	hasStagedChanges bool,
+	treeDepth int,
+	visualDepth int,
+	submoduleConfigs []*models.SubmoduleConfig,
+	node *filetree.Node[models.File],
+) string {
+	name := fileNameAtDepth(node, treeDepth)
+	output := ""
 
-	restColor := style.FgGreen
+	var nameColor style.TextStyle
-	if name == diffName {
+
-		restColor = theme.DiffTerminalColor
+	file := node.File
-	} else if file == nil && hasStagedChanges && hasUnstagedChanges {
+
-		restColor = partiallyModifiedColor
+	indentation := strings.Repeat("  ", visualDepth)
-	} else if hasUnstagedChanges {
+
-		restColor = theme.UnstagedChangesColor
+	if hasStagedChanges && !hasUnstagedChanges {
+		nameColor = style.FgGreen
+	} else if hasStagedChanges {
+		nameColor = style.FgYellow
+	} else {
+		nameColor = theme.DefaultTextColor
 	}
 
-	output := ""
+	if file == nil {
-	if file != nil {
+		output += indentation + ""
-		// this is just making things look nice when the background attribute is 'reverse'
+		arrow := EXPANDED_ARROW
-		firstChar := file.ShortStatus[0:1]
+		if isCollapsed {
-		firstCharCl := style.FgGreen
+			arrow = COLLAPSED_ARROW
-		if firstChar == "?" {
-			firstCharCl = theme.UnstagedChangesColor
-		} else if firstChar == " " {
-			firstCharCl = restColor
 		}
 
-		secondChar := file.ShortStatus[1:2]
+		arrowStyle := nameColor
-		secondCharCl := theme.UnstagedChangesColor
-		if secondChar == " " {
-			secondCharCl = restColor
-		}
 
-		output = firstCharCl.Sprint(firstChar)
+		output += arrowStyle.Sprint(arrow) + " "
-		output += secondCharCl.Sprint(secondChar)
+	} else {
-		output += restColor.Sprint(" ")
+		// 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, nameColor) + nameColor.Sprint(" ")
 	}
 
 	isSubmodule := file != nil && file.IsSubmodule(submoduleConfigs)
@@ -162,10 +153,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) + nameColor.Sprint(" ")
 	}
 
-	output += restColor.Sprint(utils.EscapeSpecialChars(name))
+	output += nameColor.Sprint(utils.EscapeSpecialChars(name))
 
 	if isSubmodule {
 		output += theme.DefaultTextColor.Sprint(" (submodule)")
@@ -174,31 +165,76 @@ func getFileLine(hasUnstagedChanges bool, hasStagedChanges bool, name string, di
 	return output
 }
 
-func getCommitFileLine(name string, diffName string, commitFile *models.CommitFile, status patch.PatchStatus) string {
+func formatFileStatus(file *models.File, restColor style.TextStyle) string {
-	var colour style.TextStyle
+	firstChar := file.ShortStatus[0:1]
-	if diffName == name {
+	firstCharCl := style.FgGreen
-		colour = theme.DiffTerminalColor
+	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
+
+	switch status {
+	case patch.WHOLE:
+		nameColor = style.FgGreen
+	case patch.PART:
+		nameColor = style.FgYellow
+	case patch.UNSELECTED:
+		nameColor = theme.DefaultTextColor
+	}
+
+	if isDirectory {
+		arrow := EXPANDED_ARROW
+		if isCollapsed {
+			arrow = COLLAPSED_ARROW
+		}
+
+		output += nameColor.Sprint(arrow) + " "
 	} else {
+		var symbol string
+		symbolStyle := nameColor
+
 		switch status {
 		case patch.WHOLE:
-			colour = style.FgGreen
+			symbol = "●"
 		case patch.PART:
-			colour = style.FgYellow
+			symbol = "◐"
 		case patch.UNSELECTED:
-			colour = theme.DefaultTextColor
+			symbol = commitFile.ChangeStatus
+			symbolStyle = getColorForChangeStatus(symbol)
 		}
-	}
 
-	output := ""
+		output += symbolStyle.Sprint(symbol) + " "
+	}
 
 	name = utils.EscapeSpecialChars(name)
-	if commitFile != nil {
-		output += getColorForChangeStatus(commitFile.ChangeStatus).Sprint(commitFile.ChangeStatus) + " "
-	}
-
 	isSubmodule := false
 	isLinkedWorktree := false
-	isDirectory := commitFile == nil
 
 	if icons.IsIconEnabled() {
 		icon := icons.IconForFile(name, isSubmodule, isLinkedWorktree, isDirectory)
@@ -206,7 +242,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
 }
 

pkg/gui/presentation/files_test.go

@@ -74,7 +74,7 @@ M  file1
 			for _, path := range s.collapsedPaths {
 				viewModel.ToggleCollapsed(path)
 			}
-			result := RenderFileTree(viewModel, "", nil)
+			result := RenderFileTree(viewModel, nil)
 			assert.EqualValues(t, s.expected, result)
 		})
 	}
@@ -141,7 +141,7 @@ M file1
 				},
 			)
 			patchBuilder.Start("from", "to", false, false)
-			result := RenderCommitFileTree(viewModel, "", patchBuilder)
+			result := RenderCommitFileTree(viewModel, patchBuilder)
 			assert.EqualValues(t, s.expected, result)
 		})
 	}