From fd93da799d3c8529ac808b201b35550dfacf1151 Mon Sep 17 00:00:00 2001
From: Christopher Allen Lane <chris@chris-allen-lane.com>
Date: Fri, 26 Aug 2022 13:14:03 -0400
Subject: [PATCH] fix(sheets): cheatsheets in submodules (#694)

Resolve an issue whereby cheatsheets contained within `git` submodules
were ignored due to a regression introduced in `4.3.1`.
---
 internal/sheets/gitdir.go | 72 +++++++++++++++++++++++++++++++++++++++
 internal/sheets/load.go   | 14 +++-----
 2 files changed, 77 insertions(+), 9 deletions(-)
 create mode 100644 internal/sheets/gitdir.go

diff --git a/internal/sheets/gitdir.go b/internal/sheets/gitdir.go
new file mode 100644
index 0000000..74c3f9a
--- /dev/null
+++ b/internal/sheets/gitdir.go
@@ -0,0 +1,72 @@
+package sheets
+
+import (
+	"fmt"
+	"os"
+	"strings"
+)
+
+// isGitDir returns `true` if `path` is within a `.git` directory, or `false`
+// otherwise
+func isGitDir(path string) (bool, error) {
+
+	/*
+		A bit of context is called for here, because this functionality has
+		previously caused a number of tricky, subtle bugs.
+
+		Fundamentally, here we are simply trying to avoid walking over the
+		contents of the `.git` directory. Doing so potentially makes
+		hundreds/thousands of needless syscalls, and can noticeably harm
+		performance on machines with slow disks.
+
+		The earliest effort to solve this problem involved simply returning
+		`fs.SkipDir` when the cheatsheet file path began with `.`, signifying a
+		hidden directory. This, however, caused two problems:
+
+		1. The `.cheat` directory was ignored
+		2. Cheatsheets installed by `brew` (which were by default installed to
+		`~/.config/cheat`) were ignored
+
+		See: https://github.com/cheat/cheat/issues/690
+
+		To remedy this, the exclusion criteria were narrowed, and the search
+		for a literal `.` was replaced with a search for a literal `.git`.
+		This, however, broke user installations that stored cheatsheets in
+		`git` submodules, because such an installation would contain a `.git`
+		file that pointed to the upstream repository.
+
+		See: https://github.com/cheat/cheat/issues/694
+
+		Accounting for all of the above, we are now searching for the presence
+		of a `.git` literal string in the cheatsheet file path. If it is not
+		found, we continue to walk the directory, as before.
+
+		If it *is* found, we determine if `.git` refers to a file or directory,
+		and only stop walking the path in the latter case.
+	*/
+
+	// determine if the literal string `.git` appears within `path`
+	pos := strings.Index(path, ".git")
+
+	// if it does not, we know for certain that we are not within a `.git`
+	// directory.
+	if pos == -1 {
+		return false, nil
+	}
+
+	// If `path` does contain the string `.git`, we need to determine if we're
+	// inside of a `.git` directory, or if `path` points to a cheatsheet that's
+	// stored within a `git` submodule.
+	//
+	// See: https://github.com/cheat/cheat/issues/694
+
+	// truncate `path` to the occurrence of `.git`
+	f, err := os.Stat(path[:pos+4])
+	if err != nil {
+		return false, fmt.Errorf("failed to stat path %s: %v", path, err)
+	}
+
+	// return true or false depending on whether the truncated path is a
+	// directory
+	return f.Mode().IsDir(), nil
+}
diff --git a/internal/sheets/load.go b/internal/sheets/load.go
index 4442b3e..833d111 100644
--- a/internal/sheets/load.go
+++ b/internal/sheets/load.go
@@ -52,15 +52,11 @@ func Load(cheatpaths []cp.Cheatpath) ([]map[string]sheet.Sheet, error) {
 				// Don't walk the `.git` directory. Doing so creates
 				// hundreds/thousands of needless syscalls and could
 				// potentially harm performance on machines with slow disks.
-				//
-				// NB: We _do_ want to walk hidden directories, however, so we
-				// should not constrain this further (perhaps to include all
-				// hidden directories). In the wild, many users appear to store
-				// cheatsheets in a `.config` directory (seemingly a default
-				// behavior of `brew`), and `cheat` explicitly supports a
-				// local `.cheat` directory. Constraining further here will
-				// break those use-cases - and has done so in the past!
-				if strings.Contains(path, ".git") {
+				skip, err := isGitDir(path)
+				if err != nil {
+					return fmt.Errorf("failed to identify .git directory: %v", err)
+				}
+				if skip {
 					return fs.SkipDir
 				}