Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sam rm: fixed command behavior when combine -d/-s/-f options + enabled '.' and '..' inputs #580

Merged
merged 6 commits into from
Aug 29, 2016
Merged

sam rm: fixed command behavior when combine -d/-s/-f options + enabled '.' and '..' inputs #580

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe7d860
sam rm: fixed remove of items when combine -d/-f/-s options
Aug 19, 2016
941c803
sam rm: refactored how to check if the input is a directory
Aug 19, 2016
affd728
sam rm: improved error logged when cannot remove a directory
Aug 19, 2016
ac249f4
sam rm: can use '.' or '..' as input
Aug 19, 2016
805ca01
sam rm: added try/catch around browse instructions
Aug 19, 2016
2eb9ae2
sam rm: improved the help of --directories/--sequences/--files options
Aug 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 33 additions & 25 deletions applications/sam/sam_rm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ def _removeItem(self, item, args):
# sam-rm --dry-run
if not args.dryRun:
os.rmdir(absoluteFolderPath)
except OSError:
self.logger.error('You cannot remove a folder which contains elements like this. If you still want to, see "-R" option.')
except OSError as e:
self.logger.debug(e)
self.logger.error('You cannot remove the folder "' + absoluteFolderPath + '", probably because it is not empty. If you still want to, try the "-R" option.')
return 1
return 0

Expand Down Expand Up @@ -113,26 +114,32 @@ def _removeItems(self, items, args, detectionMethod, filters):

for item in sorted(items):
itemType = item.getType()
toRemove = True

# sam-rm -d
if args.directories and itemType != sequenceParser.eTypeFolder:
toRemove = False

# sam-rm -f
if args.files and itemType != sequenceParser.eTypeFile:
toRemove = False
toRemove = False

# sam-rm -s
if args.sequences and itemType != sequenceParser.eTypeSequence:
toRemove = False
# sam-rm default case: remove all items
if not args.directories and not args.files and not args.sequences:
toRemove = True
else:
# sam-rm -d
if args.directories and itemType == sequenceParser.eTypeFolder:
toRemove = True
# sam-rm -f
elif args.files and itemType == sequenceParser.eTypeFile:
toRemove = True
# sam-rm -s
elif args.sequences and itemType == sequenceParser.eTypeSequence:
toRemove = True

# sam-rm -R
if args.recursive and itemType == sequenceParser.eTypeFolder:
subFolder = os.path.join(item.getFolder(), item.getFilename())
self.logger.debug('Browse in sub folder"' + subFolder + '" with the following filters: ' + str(filters))
newItems = sequenceParser.browse(subFolder, detectionMethod, filters)
self._removeItems(newItems, args, detectionMethod, filters)
try:
subFolder = os.path.join(item.getFolder(), item.getFilename())
self.logger.debug('Browse in sub folder"' + subFolder + '" with the following filters: ' + str(filters))
newItems = sequenceParser.browse(subFolder, detectionMethod, filters)
Copy link
Member

Choose a reason for hiding this comment

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

It seems that this is not recursive.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because of the potential exception?

Copy link
Member

Choose a reason for hiding this comment

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

sequenceParser.browse browse the input folder but not recursively. And I don't see any code around to make the recursion.

Copy link
Member Author

Choose a reason for hiding this comment

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

The instruction just below:

self._removeItems(newItems, args, detectionMethod, filters)

Copy link
Member

Choose a reason for hiding this comment

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

Oh yes, you're right! I didn't see that it's the same function.

self._removeItems(newItems, args, detectionMethod, filters)
except IOError as e:
self.logger.error(e)
error = 1

if toRemove:
# store folder and delete it after all other elements
Expand Down Expand Up @@ -171,10 +178,6 @@ def run(self, parser):
self.logger.error('You cannot cumulate multiple options to specify the range of sequence.')
exit(1)

if '.' in args.inputs or '..' in args.inputs:
self.logger.error('You cannot remove folders "." or "..".')
exit(1)

# sam-rm -a
detectionMethod = sequenceParser.eDetectionDefault
if args.all:
Expand All @@ -195,7 +198,7 @@ def run(self, parser):
items = []

# input is a directory
if not os.path.basename(inputPath):
if os.path.isdir(inputPath):
items.append(sequenceParser.Item(sequenceParser.eTypeFolder, inputPath))
# else browse directory with a filter, to find the corresponding Item
else:
Expand All @@ -208,8 +211,13 @@ def run(self, parser):
filterToBrowse.extend(filters)
filterToBrowse.append(os.path.basename(inputPath))
# browse
self.logger.debug('Browse in "' + pathToBrowse + '" with the following filters: ' + str(filterToBrowse))
items = sequenceParser.browse(pathToBrowse, detectionMethod, filterToBrowse)
try:
self.logger.debug('Browse in "' + pathToBrowse + '" with the following filters: ' + str(filterToBrowse))
items = sequenceParser.browse(pathToBrowse, detectionMethod, filterToBrowse)
except IOError as e:
self.logger.error(e)
error = 1
continue

# print error if no items were found
if len(items) == 0:
Expand Down