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

@@ -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,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,39 +173,93 @@ 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)
 		paint := color.C256(icon.Color, false)
 		output += paint.Sprint(icon.Icon) + " "
 	}
 
-	output += colour.Sprint(name)
+	output += nameColor.Sprint(name)
 	return output
 }