Skip to content

Conversation

@fazzabi
Copy link

@fazzabi fazzabi commented Aug 13, 2025

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)

- Add --remove-orphaned flag to sync command for safe cleanup
- Add remove_orphaned config option in mani.yaml
- Implement git-repository-only detection (improved from directory blacklist approach)
- Include user confirmation with preview before deletion
- Add comprehensive unit tests and documentation
- Only removes directories containing .git folders that aren't in config
@fazzabi fazzabi marked this pull request as ready for review August 13, 2025 14:12
@fazzabi fazzabi changed the title feat: add --remove-orphaned flag with git-repository-only detection feat: add --remove-orphaned flag to sync command Aug 13, 2025
@fazzabi fazzabi changed the title feat: add --remove-orphaned flag to sync command feat: add --remove-orphaned flag with git-repository-only detection Aug 13, 2025
@fazzabi fazzabi changed the title feat: add --remove-orphaned flag with git-repository-only detection feat: add --remove-orphaned flag to mani sync command Aug 13, 2025
@fazzabi
Copy link
Author

fazzabi commented Aug 13, 2025

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

- Remove redundant syncFlags.RemoveOrphaned check (comment 1)
- Remove unused projects parameter from RemoveOrphanedProjects (comment 2)
- Remove test-only removeOrphanedProjectsWithConfirm function (comment 3)
- Fix filtering bug: orphaned cleanup now considers all projects, not filtered ones
- Simplify function signature and always require user confirmation
@fazzabi fazzabi requested a review from alajmo August 19, 2025 13:16
@fazzabi
Copy link
Author

fazzabi commented Aug 19, 2025

@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
Copy link
Author

fazzabi commented Sep 3, 2025

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
Copy link
Author

fazzabi commented Sep 22, 2025

@alajmo please ⬆️

@alajmo
Copy link
Owner

alajmo commented Oct 7, 2025

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.

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