Skip to content

fix: code quality improvements and runValidation tests#22

Open
oxGrad wants to merge 2 commits intomainfrom
fix/code-quality
Open

fix: code quality improvements and runValidation tests#22
oxGrad wants to merge 2 commits intomainfrom
fix/code-quality

Conversation

@oxGrad
Copy link
Copy Markdown
Owner

@oxGrad oxGrad commented Apr 23, 2026

Summary

  • Bug: classifyTarget silently discarded a filepath.EvalSymlinks error — now falls back to the raw source path
  • Bug: validate called os.Exit inside RunE, bypassing Cobra's error handling — replaced with an ExitError type that carries the exit code out to Execute()
  • Bug: all unchecked fmt.Fprintf calls now use _, _ = (errcheck)
  • Design: loadConfig now uses config.DefaultKnotfilePath (~/.dotfiles/Knotfile, overridable via $KNOT_DIR) instead of walking upward from CWD
  • Design: config package gains KnownOS, DefaultDir, DefaultKnotfilePath, KnotfileName, EnvKnotDir; Load documents the ~/ path-resolution asymmetry
  • Quality: Execute() sets SilenceErrors = true and prints errors once with an Error: prefix (fixes double-print)
  • Quality: --config flag description now mentions the default path and KNOT_DIR
  • Quality: resolvePackageArgs simplified — duplicate package-name validation removed (linker already validates); --all output is now sorted alphabetically
  • Quality: validate refactored — runValidation() extracted for testability, uses config.KnownOS, sorts packages alphabetically, sets SilenceUsage = true
  • Quality: Status() sorts package names before printing
  • Quality: ShouldIgnore doc comment clarifies base-name-only matching
  • Tests: 14 unit tests for runValidation covering all validation paths

Test plan

  • go test ./... passes
  • golangci-lint run ./... reports 0 issues
  • knot status output is alphabetically sorted
  • knot validate exits 2 on warnings-only, 1 on errors, 0 on clean
  • knot tie unknown-pkg prints Error: unknown package exactly once
  • KNOT_DIR=/tmp/dots knot status reads /tmp/dots/Knotfile
  • knot --config /path/to/Knotfile tie pkg still works

https://claude.ai/code/session_018s5iF8MKZAobUjAuR1AHJ4


Generated by Claude Code

claude added 2 commits April 20, 2026 04:36
Bugs fixed:
- classifyTarget: handle EvalSymlinks error instead of silently
  discarding it (fall back to raw source path)
- validate: replace os.Exit inside RunE with ExitError type so
  Execute() controls the exit code — fixes testability and double-print
- All unchecked fmt.Fprintf calls now use _, _ = (errcheck)

Design:
- loadConfig: use DefaultKnotfilePath (~/.dotfiles/Knotfile, $KNOT_DIR)
  instead of upward CWD walk (FindConfigFile)
- config: add KnownOS, DefaultDir, DefaultKnotfilePath, KnotfileName,
  EnvKnotDir; document ~/path resolution asymmetry in Load
- resolver: document that ShouldIgnore matches base name only

Code quality:
- Execute: set SilenceErrors=true, print errors once with "Error:" prefix
- --config flag description now mentions $HOME/.dotfiles/Knotfile and KNOT_DIR
- resolvePackageArgs: remove duplicate package-name validation (linker
  already validates); sort --all output alphabetically
- validate: extract runValidation() for testability; use config.KnownOS;
  sort packages alphabetically; set SilenceUsage=true
- Status: sort package names alphabetically before printing

https://claude.ai/code/session_018s5iF8MKZAobUjAuR1AHJ4
14 cases covering all validation paths:
- empty packages → warning
- valid package (absolute + ~/... source) → clean
- missing source / target / both fields
- source is a file not a directory
- source path does not exist
- known and unknown condition.os values
- nil and empty condition
- valid and invalid ignore glob patterns
- ~/... home expansion via the home argument
- alphabetical sort of error output across packages

https://claude.ai/code/session_018s5iF8MKZAobUjAuR1AHJ4
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.

2 participants