From 908b19015fc4bffc54b1d554b83fe6c1c93b20ee Mon Sep 17 00:00:00 2001
From: Braydon Kains <93549768+braydonk@users.noreply.github.com>
Date: Tue, 6 Jun 2023 22:11:26 -0400
Subject: [PATCH] yamlfmt: fix standard mode exclude bug (#124)

Due to a misunderstanding on my part, I swapped standard filepath
collection mode to calculate exclusions using absolute paths. This
worked because of the way tests were written, but it does not work in
most real world scenarios and it slipped by. I will be able to catch
this better when I implement proper integration testing.

This PR removes the absolute paths from the calculation, and adds a doc
entry to recommend that paths are always specified relative to the
command working directory.
---
 docs/paths.md          |  4 ++++
 path_collector.go      |  9 ++-------
 path_collector_test.go | 24 ++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/docs/paths.md b/docs/paths.md
index 768bb17..23aa3ba 100644
--- a/docs/paths.md
+++ b/docs/paths.md
@@ -9,3 +9,7 @@ In standard path mode, the you can specify a file or directory path directly. If
 ## Doublestar
 
 In Doublestar mode, paths are specified using the format explained in the [doublestar](https://github.com/bmatcuk/doublestar) package. It is almost identical to bash and git's style of glob pattern specification.
+
+## Include and Exclude
+
+In both modes, `yamlfmt` will allow you to configure include and exclude paths. These can be paths to files in Standard or Doublestar modes, paths to directories in Standard mode, and valid doublestar patterns in Doublestar mode. These paths should be specified **relative to the working directory of `yamlfmt`**. They will work as absolute paths if both the includes and excludes are specified as absolute paths or if both are relative paths, however it will not work as expected if they are mixed together. It usually easier to reason about includes and excludes when always specifying both as relative paths from the directory `yamlfmt` is going to be run in.
\ No newline at end of file
diff --git a/path_collector.go b/path_collector.go
index e8b265b..d943dab 100644
--- a/path_collector.go
+++ b/path_collector.go
@@ -52,19 +52,14 @@ func (c *FilepathCollector) CollectPaths() ([]string, error) {
 			continue
 		}
 
-		absExclPath, err := filepath.Abs(exclPath)
-		if err != nil {
-			return nil, err
-		}
-
 		if info.IsDir() {
 			for foundPath := range pathsFoundSet {
-				if strings.HasPrefix(foundPath, absExclPath) {
+				if strings.HasPrefix(foundPath, exclPath) {
 					pathsToFormat.Remove(foundPath)
 				}
 			}
 		} else {
-			pathsToFormat.Remove(absExclPath)
+			pathsToFormat.Remove(exclPath)
 		}
 	}
 
diff --git a/path_collector_test.go b/path_collector_test.go
index 33ef003..2963213 100644
--- a/path_collector_test.go
+++ b/path_collector_test.go
@@ -129,7 +129,8 @@ func TestFilepathCollector(t *testing.T) {
 			},
 		},
 		{
-			name: "exclude directory",
+			name:            "exclude directory",
+			changeToTempDir: true,
 			files: []tempfile.Path{
 				{FileName: "x.yml"},
 				{FileName: "y.yml"},
@@ -159,6 +160,25 @@ func TestFilepathCollector(t *testing.T) {
 				"a/x.yaml": {},
 			},
 		},
+		{
+			name: "don't get files with wrong extension",
+			files: []tempfile.Path{
+				{FileName: "x.yml"},
+				{FileName: "y.yaml"},
+				{FileName: "z.json"},
+			},
+			includePatterns: testPatterns{
+				{pattern: ""}, // with the test this functionally means the whole temp dir
+			},
+			extensions: []string{
+				"yaml",
+				"yml",
+			},
+			expectedFiles: collections.Set[string]{
+				"x.yml":  {},
+				"y.yaml": {},
+			},
+		},
 	}.runAll(t, useFilepathCollector)
 }
 
@@ -338,7 +358,7 @@ func (tc testCase) run(t *testing.T, makeCollector makeCollectorFunc) {
 		collector := makeCollector(tc, tempPath)
 		paths, err := collector.CollectPaths()
 		if err != nil {
-			t.Fatalf("CollectDoublestarPathsToFormat failed: %v", err)
+			t.Fatalf("Test case failed: %v", err)
 		}
 
 		filesToFormat := collections.Set[string]{}