mani icon indicating copy to clipboard operation
mani copied to clipboard

feat: add --remove-orphaned flag to mani sync command

Open fazzabi opened this issue 4 months ago • 5 comments

What's Changed

This PR implements the --remove-orphaned flag for the mani sync command, addressing the feature request in issue #108. The implementation provides a safe way to clean up project directories that are no longer defined in the mani configuration.

Key Features:

  • Safety-first approach: Only removes directories containing .git folders (git repositories)
  • User confirmation: Shows a preview of what will be deleted and requires explicit confirmation
  • Configuration option: Supports remove_orphaned: true in mani.yaml for permanent enablement
  • Comprehensive testing: Unit tests cover edge cases and integration scenarios

Example Usage:

# Use flag to remove orphaned projects with confirmation
mani sync --remove-orphaned

# Or enable permanently in mani.yaml
remove_orphaned: true

Technical Description

Core Implementation:

  • Git-repository-only detection: The implementation uses os.Stat(filepath.Join(path, ".git")) to identify git repositories, ensuring non-git directories are never removed
  • Path resolution: Uses core.GetAbsolutePath() for consistent path handling across different project configurations
  • User confirmation: Implements bufio.NewReader(os.Stdin) for interactive confirmation with clear preview of deletion targets
  • Error handling: Comprehensive error wrapping with fmt.Errorf() and graceful failure modes

Safety Measures:

  1. Whitelist approach: Only git repositories are considered for removal
  2. Explicit confirmation: Defaults to 'No' unless user explicitly types 'y' or 'yes'
  3. Preview display: Shows relative paths of directories that will be removed
  4. Graceful cancellation: Operation can be cancelled at any point without side effects

Architecture:

  • RemoveOrphanedProjects() - Public API with confirmation
  • removeOrphanedProjectsWithConfirm() - Internal function supporting test scenarios
  • Flag integration follows existing patterns in cmd/sync.go
  • Configuration parsing in core/dao/config.go with safe defaults

Testing Strategy:

  • Unit tests verify git-repository-only detection logic
  • Edge case coverage: non-git directories, hidden directories, active projects
  • Integration test ensures flag works in real scenarios
  • Mock-friendly design allows testing without user interaction

Before opening a Pull Request you should read and agreed to the Contributor Code of Conduct (see CONTRIBUTING.md)

fazzabi avatar Aug 13 '25 14:08 fazzabi

@alajmo could you please review the PR when you have time? thanks

fazzabi avatar Aug 13 '25 15:08 fazzabi

@alajmo Thank you for the thorough review! I've addressed all three comments in commit 792ad41:

  1. Removed redundant flag check - Now using only *config.RemoveOrphaned as suggested
  2. Cleaned up function signature - Removed unused projects parameter
  3. Eliminated test-only function - Consolidated into single function with built-in confirmation

The functionality has been thoroughly tested manually and all existing tests pass. The feature now has a cleaner implementation while maintaining the same safety guarantees and user experience.

Ready for re-review! 🚀

fazzabi avatar Aug 19 '25 13:08 fazzabi

Hi @alajmo ! Just a gentle reminder to review this PR when you have a moment. Let me know if you have any questions or need more details. Thank you!

fazzabi avatar Sep 03 '25 15:09 fazzabi

@alajmo please ⬆️

fazzabi avatar Sep 22 '25 07:09 fazzabi

Sorry for late response, just some minor things I've found (I'll be much faster reviewing this time!):

  1. move var orphanedPaths []string before usage, no need to declare it if we return early
  2. We need to handle nested directories, it's quite common to use sub-directories for storing projects (I use it myself). We don't have to check every directory, could be enough with going into directories that don't have a .git folder.

alajmo avatar Oct 07 '25 19:10 alajmo