Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Conversation

kuba--
Copy link
Contributor

@kuba-- kuba-- commented Aug 24, 2018

Signed-off-by: kuba-- kuba@sourced.tech
Fixes #895

When CleanOptions.Dir is set on true then an empty dir (with subdirs) could be removed (this is what git clean -f -d . does).

@kuba-- kuba-- requested review from mcuadros and smola and removed request for mcuadros and smola August 24, 2018 22:49
@kuba--
Copy link
Contributor Author

kuba-- commented Aug 26, 2018

@onodera-punpun - Could you check if this fix works for you?

@ajnavarro
Copy link
Contributor

@kuba-- could you add a test reproducing the error on #895, please?

@kuba--
Copy link
Contributor Author

kuba-- commented Aug 27, 2018

@ajnavarro - done. I've added a test to check if empty dirs are cleaned, as well.

@kuba-- kuba-- requested a review from a team August 27, 2018 09:08
worktree.go Outdated
// skip the path.
if relativePath != filepath.Base(relativePath) && !opts.Dir {
continue
var clean func(string, []os.FileInfo) error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this function to a member function called doClean

worktree.go Outdated
if status.Worktree == Untracked {
absPath := filepath.Join(w.Filesystem.Root(), relativePath)
if err := os.Remove(absPath); err != nil {
if opts.Dir {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also will be nice move this code to a function something like doCleanDirectories

kuba-- added 2 commits August 29, 2018 14:38
Signed-off-by: kuba-- <kuba@sourced.tech>
Signed-off-by: kuba-- <kuba@sourced.tech>
@kuba--
Copy link
Contributor Author

kuba-- commented Aug 29, 2018

@mcuadros - PR refined and rebased.

@mcuadros mcuadros merged commit 34b101e into src-d:master Aug 29, 2018
@kuba-- kuba-- deleted the fix-895/clean-dir branch August 29, 2018 20:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants