feat(command): add force flag for files rm #5555
Conversation
core/commands/files.go
Outdated
| @@ -1002,6 +1002,7 @@ Remove files or directories. | |||
| }, | |||
| Options: []cmdkit.Option{ | |||
| cmdkit.BoolOption("recursive", "r", "Recursively remove directories."), | |||
| cmdkit.BoolOption("force", "Allow to remove anything else"), | |||
There was a problem hiding this comment.
What do you think of forcibly remove target at path; implies -r for directories as a description?
overbool
force-pushed
the
feat/add-force-flag-for-files-rm
branch
from
16b8423
to
9895922
Compare
License: MIT Signed-off-by: Overbool <[email protected]>
overbool
force-pushed
the
feat/add-force-flag-for-files-rm
branch
from
9895922
to
bd23e7d
Compare
overbool
changed the title
License: MIT Signed-off-by: Overbool <[email protected]>
|
@schomatis could u help me review this pr? But I have a question about command option: when i add |
|
The but that's ok, I actually prefer to just have the long explicit |
| switch childi.(type) { | ||
| dashr, _, _ := req.Option("r").Bool() | ||
|
|
||
| switch child.(type) { | ||
| case *mfs.Directory: |
There was a problem hiding this comment.
nit: we can now turn this single-case switch into an if
| @@ -629,6 +629,20 @@ test_files_api() { | |||
| test_expect_success "repo gc $EXTRA" ' | |||
| ipfs repo gc | |||
| ' | |||
|
|
|||
| # test rm | |||
There was a problem hiding this comment.
I'd like to add a test with an actual corrupted node, which could just be an empty proto-node, but I'm not sure how could we add that to MFS, ipfs files cp does check the type:
ipfs object new
# QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n
ipfs files cp /ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n /corrupted
# Error: cp: cannot flush the created file /corrupted: proto: required field "Type" not set
There was a problem hiding this comment.
Oh, turn off flush:
ipfs files -f=0 cp /ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n /corrupted
ipfs files rm /corrupted
# Error: proto: required field "Type" not set
There was a problem hiding this comment.
OT: What's the difference of adding a file in MFS without flushing? I still don't know how that works (and I'm supposedly the one in charge of reviewing the MFS API 😬)
There was a problem hiding this comment.
Thinking about this some more, after adding the checks to PutNode this won't work anyways (even with flush turned off), so scratch that, I'm fine with the existing tests.
Refer to: #5538 (comment), #5554
License: MIT
Signed-off-by: Overbool [email protected]