Skip to content

fix: respect directory boundaries in standard excludes - #325

Open
Shubham-Padkonde wants to merge 1 commit into
google:mainfrom
Shubham-Padkonde:fix/exclude-directory-boundaries
Open

Shubham-Padkonde wants to merge 1 commit into
google:mainfrom
Shubham-Padkonde:fix/exclude-directory-boundaries

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Standard directory exclusions currently use a string prefix: excluding vendor also drops vendor-local/config.yaml and vendor.yaml from the formatting set. A directory spelled as vendor/. has the opposite problem and fails to exclude its contents.

Use filepath.Rel to check lexical directory containment, rejecting parent-relative paths. File exclusions and the other matching modes are unchanged. Four regression cases cover a directory, a trailing separator, a dot segment, and the root directory.

Validation (Windows, Go 1.27.1):

  • Two new cases fail on upstream a74383c; all four pass with the patch.
  • Built and ran both upstream and patched CLI binaries against real files: upstream skips the sibling files; the patch formats them and leaves the excluded directory untouched.
  • Full go test -p 1 ./... has existing Windows path/line-ending failures. Compared failing test names against the original production source with the new tests retained: no failures unique to the patch; only the two new regression cases fail additionally upstream.
  • go vet over the same package set as the Makefile target (excluding pkg/yaml): passed. A broader vet invocation also examined vendored YAML test fixtures and reported their deliberately malformed struct tags; that package is excluded by the project's target.
  • Repository addlicense check and gofmt/diff checks passed.
  • Linux CI, race tests, and the full command integration suite were not run locally.

Prepared and validated with OpenAI Codex assistance. No CLA was signed by the agent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant