fixes to file.go in codewalker for issues #133 and #404 in SCC repository, by m39johnsonm · Pull Request #22 · boyter/gocodewalker

m39johnsonm · 2026-04-20T23:41:04Z

SCC would only read .gitignore files, but not .git/info/exclude, in file.go we explicitly check for the git/info using GIT_DIR environment variable , and we add it to the .gitignore. This fix was tested originally by creating a test directory, in our SCC fork, and reproducing the git/info/exclude, the user had, where the scc properly ignored the .git/info/exclude, and did not count it.

RichardSimison worked on issues 404, where Linux and windows were outputting different results. The issues that was identified, was Linux and windows have different slashes, so when filepath.join was used it produced backslashes on windows, causing .gitignores to fail. The fix was to apply filepath.ToSlash to all paths before passing
them to MatchIsDir and gitignore.New. Tests were created to check that windows styled line endings are parsed correctly with .gitignore files

when gocodewalker, walks through directories the .gitignore files were respected but .git/info/exclude was not now when walking a directory if .git/info/exists it's read, and ignored in the scc count Fixes boyter/scc#133

…c#404) filepath.Join produces backslashes on Windows, causing gitignore rules to silently fail. Apply filepath.ToSlash to all paths before passing them to MatchIsDir and gitignore.New. Adds regression tests for CRLF .gitignore files and path normalization.

boyter · 2026-04-20T23:43:09Z

Oh neat. I have been working on this as well, but you having done it already is a great benefit. Looking though it now.

Copilot

Pull request overview

This PR aims to improve ignore handling consistency across platforms and add support for Git’s global exclude file, addressing SCC issues #133 and #404.

Changes:

Normalize paths to forward-slash form when matching ignores to reduce Windows/Linux discrepancies.
Add support for reading $GIT_DIR/info/exclude (or .git/info/exclude) and applying it during walking.
Add tests for CRLF .gitignore parsing and for path normalization behavior.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`file.go`	Normalizes walked paths before ignore matching; adds loading of `.git/info/exclude`.
`file_test.go`	Adds tests for CRLF `.gitignore` and “Windows path normalization”.
`go-gitignore/gitignore.go`	Normalizes inputs for `MatchIsDir` cache/matching.
`go-gitignore/lexer.go`	Changes CRLF end-of-line tokenization behavior.
`go-gitignore/repository.go`	Adjusts how per-directory ignore file paths are constructed.
`go.sum`	Adds new module sums (likely from updated/added test dependencies).

Comments suppressed due to low confidence (2)

go-gitignore/gitignore.go:266

MatchIsDir now normalizes both the cache key and the computed absolute path to slash form, but ignore._base is not normalized anywhere (e.g. NewWithErrors sets it via filepath.Dir using OS separators). On Windows this will break Absolute()'s strings.HasPrefix(path, i._base) check because path becomes C:/... while _base is C:\.... Consider normalizing _base to slash form at construction time (in New, NewWithErrors, and repository constructors), or alternatively avoid converting _path to slash form here and normalize only the relative portion used for pattern matching.

func (i *ignore) MatchIsDir(path string, _isdir bool) Match {
	// ensure we have the absolute path for the given file
	path = filepath.ToSlash(path) // normalize before cache lookup
	if v, ok := matchIsDirCache.Load(path); ok {
		return i.Absolute(v.(string), _isdir)
	}

	_path, _err := filepath.Abs(path)
	if _err != nil {
		i._errors(NewError(_err, Position{}))
		return nil
	}
	_path = filepath.ToSlash(_path) // ensure stored value is slash-form
	matchIsDirCache.Store(path, _path)

	// attempt to match the absolute path
	return i.Absolute(_path, _isdir)

go-gitignore/lexer.go:334

eol() consumes both \r and \n for CRLF, but after this change it only appends \n to the returned word. Since token positioning uses len(word) to compute the start column/offset, CRLF lines will now produce incorrect Position data (off by 1) for EOL tokens and downstream error reporting. Either append both runes ("\r\n") to _line or adjust the position calculation to account for the extra consumed rune.

		// carriage return - we expect to see a newline next
		case _CR:
			_next, _err = l.read()
			if _err != nil {
				return _line, _err
			} else if _next != _NEWLINE {
				l.unread(_next)
				return _line, l.err(ErrCarriageReturnError)
			}
			fallthrough

		// newline
		case _NEWLINE:
			_line = append(_line, _next)
			return _line, nil

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T23:50:14Z

+		_file := r._base + string(os.PathSeparator) +
+			filepath.FromSlash(_parent) + string(os.PathSeparator) + r._file


This replaces filepath.Join with manual path concatenation. This is more error-prone on Windows (drive roots like C:\ or UNC paths) and can introduce redundant separators/./ segments that filepath.Join normally normalizes. Unless there’s a specific bug being worked around, prefer filepath.Join(r._base, _parent, r._file) (or at least filepath.Clean the constructed path) to preserve cross-platform path semantics.

Suggested change

_file := r._base + string(os.PathSeparator) +

filepath.FromSlash(_parent) + string(os.PathSeparator) + r._file

_file := filepath.Join(r._base, _parent, r._file)

Copilot · 2026-04-20T23:50:14Z

+				gitIgnore := gitignore.New(bytes.NewReader(c), filepath.ToSlash(abs), nil)
 				gitignores = append(gitignores, gitIgnore)


Only the .gitignore ignore instance is created with a slash-normalized base (filepath.ToSlash(abs)), but other ignore instances in this function (.ignore, .gitmodules, CustomIgnore, CustomIgnorePatterns, and the new info/exclude ignore) still use abs directly. With MatchIsDir now normalizing paths to slash form, any ignore with a non-normalized base will fail the HasPrefix check on Windows and effectively stop matching. Normalize the base consistently for all gitignore.New(...) calls (or normalize inside the gitignore library constructors).

Copilot · 2026-04-20T23:50:14Z

+		if gitdir == "" {
+			gitdir = filepath.Join(directory, ".git")
+		}
+		file := filepath.Join(gitdir, "info", "exclude")
+		if content, err := os.ReadFile(file); err == nil {
+			abs, err := filepath.Abs(directory)
+			if err == nil {
+				gitExclude := gitignore.New(bytes.NewReader(content), abs, nil)
+				if gitExclude != nil {
+					gitignores = append(gitignores, gitExclude)
+				}


The $GIT_DIR/info/exclude file is being probed on every recursive directory walk, which adds an extra filesystem read per directory and can become expensive on large trees. Since the exclude file is repo-global, consider loading it once (e.g. only at iteration == 0, or only when a .git directory is present) and passing/caching the resulting GitIgnore down to child calls. Also, for consistency with the other ignore loaders, use the injected f.osReadFile/f.errorsHandler, and make the base path slash-normalized (to match the MatchIsDir normalization).

Suggested change

if gitdir == "" {

gitdir = filepath.Join(directory, ".git")

}

file := filepath.Join(gitdir, "info", "exclude")

if content, err := os.ReadFile(file); err == nil {

abs, err := filepath.Abs(directory)

if err == nil {

gitExclude := gitignore.New(bytes.NewReader(content), abs, nil)

if gitExclude != nil {

gitignores = append(gitignores, gitExclude)

}

shouldLoadGitExclude := gitdir != ""

if gitdir == "" {

gitdir = filepath.Join(directory, ".git")

if info, err := os.Stat(gitdir); err == nil && info.IsDir() {

shouldLoadGitExclude = true

}

}

if shouldLoadGitExclude {

file := filepath.Join(gitdir, "info", "exclude")

if content, err := f.osReadFile(file); err == nil {

abs, err := filepath.Abs(directory)

if err != nil {

if !f.errorsHandler(err) {

return err

}

} else {

abs = filepath.ToSlash(abs)

gitExclude := gitignore.New(bytes.NewReader(content), abs, nil)

if gitExclude != nil {

gitignores = append(gitignores, gitExclude)

}

}

} else if !errors.Is(err, fs.ErrNotExist) {

if !f.errorsHandler(err) {

return err

}

Copilot · 2026-04-20T23:50:14Z

+	if !f.IgnoreGitIgnore {
+		gitdir := os.Getenv("GIT_DIR")
+		if gitdir == "" {
+			gitdir = filepath.Join(directory, ".git")
+		}
+		file := filepath.Join(gitdir, "info", "exclude")
+		if content, err := os.ReadFile(file); err == nil {
+			abs, err := filepath.Abs(directory)
+			if err == nil {
+				gitExclude := gitignore.New(bytes.NewReader(content), abs, nil)
+				if gitExclude != nil {
+					gitignores = append(gitignores, gitExclude)
+				}
+			}
+		}


New behavior is introduced here to load and apply $GIT_DIR/info/exclude, but there’s no test exercising it. Since file_test.go already has extensive coverage for ignore handling, please add a unit test that creates a temp repo with .git/info/exclude containing patterns and asserts those files are skipped (including a case where IgnoreGitIgnore is true/false, and optionally when GIT_DIR is set).

Copilot · 2026-04-20T23:50:15Z

+func TestWindowsPathNormalization(t *testing.T) {
+	dir := t.TempDir()
+
+	os.WriteFile(filepath.Join(dir, ".gitignore"), []byte("build/\n"), 0644)
+	os.MkdirAll(filepath.Join(dir, "build"), 0755)
+	os.WriteFile(filepath.Join(dir, "build", "out.bin"), []byte("bin"), 0644)
+	os.WriteFile(filepath.Join(dir, "main.go"), []byte("package main"), 0644)


TestWindowsPathNormalization is intended to prevent Windows-specific path separator regressions, but the repo’s CI workflow runs tests only on ubuntu-latest, so this test won’t exercise the backslash-vs-slash behavior it’s named for. Consider either adding a Windows job to the GitHub Actions matrix or adjusting the test to explicitly cover mixed-separator inputs in a way that’s meaningful on non-Windows runners.

boyter · 2026-04-20T23:50:29Z

Ill need to review the fuzz tests on this one, because they are failing which is not ideal, but could be a false positive.

m39johnsonm · 2026-04-22T14:51:57Z

I apologize for seeing that our code pull requests has failed tests, I hope its an issue with my merging of two issues of the master branch on my fork, but I will run the tests on my fork, and see where we went wrong.

…or string, also made the variable name for my temp directory follow snake case

boyter · 2026-04-30T23:12:18Z

Merged. Ill have to upload all my downstream things for it, but thank you for this.

m39johnsonm and others added 2 commits April 16, 2026 15:24

BUG FIX: respects .git/info/exclude when walking directories

817ae1b

when gocodewalker, walks through directories the .gitignore files were respected but .git/info/exclude was not now when walking a directory if .git/info/exists it's read, and ignored in the scc count Fixes boyter/scc#133

pr-insights Bot added M/size Normal or medium sized change VH/complexity Very high complexity labels Apr 20, 2026

boyter requested a review from Copilot April 20, 2026 23:43

Copilot started reviewing on behalf of boyter April 20, 2026 23:44 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

boyter mentioned this pull request Apr 27, 2026

Fix bug with incorrect handling of prefixes #23

Merged

m39johnsonm added 2 commits April 30, 2026 18:48

Add tests for handling .git/info/exclude during directory walks

215ac90

Jetbrain refactor for one of my variables messed with name in the err…

b5c4679

…or string, also made the variable name for my temp directory follow snake case

boyter merged commit 013f7ac into boyter:master Apr 30, 2026
0 of 2 checks passed

Navigation Menu

fixes to file.go in codewalker for issues #133 and #404 in SCC repository, - #22

fixes to file.go in codewalker for issues #133 and #404 in SCC repository, #22
boyter merged 4 commits into
boyter:masterfrom
m39johnsonm:master

m39johnsonm commented Apr 20, 2026

Uh oh!

boyter commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

boyter commented Apr 20, 2026

Uh oh!

m39johnsonm commented Apr 22, 2026

Uh oh!

Uh oh!

boyter commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		_file := r._base + string(os.PathSeparator) +
		filepath.FromSlash(_parent) + string(os.PathSeparator) + r._file

	_file := r._base + string(os.PathSeparator) +
	filepath.FromSlash(_parent) + string(os.PathSeparator) + r._file
	_file := filepath.Join(r._base, _parent, r._file)

		gitIgnore := gitignore.New(bytes.NewReader(c), filepath.ToSlash(abs), nil)
		gitignores = append(gitignores, gitIgnore)

Conversation

m39johnsonm commented Apr 20, 2026

Uh oh!

boyter commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

boyter commented Apr 20, 2026

Uh oh!

m39johnsonm commented Apr 22, 2026

Uh oh!

Uh oh!

boyter commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants