PDA

View Full Version : [ET10x00] Recording Directory Deletion Issue 1



EMJB
04-04-16, 15:56
When you try to delete a folder from the "MovieList" screen, the "are you sure" message reports that there is one more subdirectory than actually exists e.g. 1 when there are none.

The source of the problem appears to be the

"subdirs += 1"

after
"are_you_sure = _("Do you really want to move to the trash can ?")"

statement at around line 1953 in the delete function in MovieSelection.py.

Commenting out this "subdirs += 1" statement clears this problem , but then prevents deletion of an empty directory , though it is not obvious to me why this is the case as the logic for the "if files or subdirs:" "else" situation that would then be invoked looks OK.

birdman
05-04-16, 01:17
When you try to delete a folder from the "MovieList" screen, the "are you sure" message reports that there is one more subdirectory than actually exists e.g. 1 when there are none.

The source of the problem appears to be the

"subdirs += 1"

after
"are_you_sure = _("Do you really want to move to the trash can ?")"
That statement appears to be there to include the directory you are currently pointing at in the count of subdirectories that will be deleted.

EMJB
05-04-16, 09:21
That statement appears to be there to include the directory you are currently pointing at in the count of subdirectories that will be deleted.True, but personally I wouldn't call that a subdirectory, nor include it with the count of true subdirectories. It also leads to the somewhat anomalous situation where you get a "Are you sure .." message when deleting an empty directory, but don't get one when deleting a recording!

Obviously not a major issue, but just something that could confuse the technophobes and which could easily be fixed.

Rob van der Does
05-04-16, 16:43
The 'are you sure' message is only there when you're about to delete multiple files/folders. That's because undeleting them (recovering them from the trash can) is quite a laborious job.
Deleting a recording does not invoke this question, as it's only about one file that's about to be moved to the trash can; so recovering it in case of an accidental deletion is quite easy.

EMJB
05-04-16, 19:24
The 'are you sure' message is only there when you're about to delete multiple files/folders. I suspect this is not the case due to the "subdirs += 1" - I think it always thinks there are multiple folders (i.e. main + one subfolder) even when the folder is actually empty - I can't check easily as I have modified it on my machine.

Rob van der Does
05-04-16, 20:36
Sorry, I should have said 'The 'are you sure' message is only there when you're about to delete a folder' for the reason I mentioned.

EMJB
06-04-16, 09:16
Sorry, I should have said 'The 'are you sure' message is only there when you're about to delete a folder' for the reason I mentioned. Perhaps that is the reason for the "subdirs += 1" statement.

However my original point stands - to me wrongly deleting an empty folder is a much lesser evil than wrongly deleting a recording. The way I am using my machine, all folders originate from auto-timers, and would get re-created if erroneously deleted. Thus erroneous deletion of an empty folder so it would be of no consequence at all. Is it that I am using my machine in a different way than the author of MovieSelection.py envisaged??

Rob van der Does
06-04-16, 10:03
Deleting a recording is no problem at all: it goes to the trash can and can be recovered or even be played from there.
Deleting (not empty) folders is a problem when you want to restore the folder/files, hence the 'are you sure'.

EMJB
06-04-16, 11:02
Deleting (not empty) folders is a problem when you want to restore the folder/files, hence the 'are you sure'.We still seem to at cross purposes - I agree with this statement, but what I find difficult to understand is the that apparently there is some problem if you erroneously delete an empty folder. Why do you need a "are you sure" in these circumstances?

Rob van der Does
06-04-16, 11:35
Not needed IMHO when folder is empty. But does it harm? It might needlessly complicate code when only showing when folder isn't empty.

EMJB
06-04-16, 13:19
Not needed IMHO when folder is empty. But does it harm? It might needlessly complicate code when only showing when folder isn't empty. There is already some code that looks as if it is intended to cope with the situation when the folder is empty, and indeed removing the "subdirs += 1" statement appears to invoke it, removing that statement gets rid of the "are you sure for" for empty directories, BUT unfortunately the directory doesn't actually get deleted!

Clearly this is not a major issue, but coupled with the report of the non-existent sub-directory, is the sort of niggling problem that makes life difficult at first for those migrating to OpenVix (particularly for technophobes). I don't see myself as being able to address the sort of problems that birdman is addressing, but would like to contribute by making things easier for the newbie, so I will have a better look at the code when I have some time, to see if I can fix that. Perhaps the experienced gained by fixing minor issues will enable me to address more important ones later.

EMJB
06-04-16, 15:00
Further investigation showed my previous understanding of the code was slightly adrift, but all that was needed was to copy the code from below where one is trying to delete from the trash can. Changing the delete function to start:

def delete(self, *args):
item = self.getCurrentSelection()
if not item or args and (not args[0]):
# cancelled by user (passing any arg means it's a dialog return)
return
current = item[0]
info = item[1]
cur_path = os.path.realpath(current.getPath())
if not os.path.exists(cur_path):
# file does not exist.
return
st = os.stat(cur_path)
name = info and info.getName(current) or _("this recording")
are_you_sure = ""
pathtest = info and info.getName(current)
if not pathtest:
return
if item and isTrashFolder(item[0]):
# Red button to empty trashcan...
self.purgeAll()
return
if current.flags & eServiceReference.mustDescent:
files = 0
subdirs = 0
if '.Trash' not in cur_path and config.usage.movielist_trashcan.value:
if isFolder(item):
are_you_sure = _("Do you really want to move to the trash can ?")
# subdirs += 1 Causes spurious subdirectory to be reported, and hence requires are you sure for empty directories!
# However with this change but without other changes below an empty directory is not deleted
else:
args = True
if args:
trash = Tools.Trashcan.createTrashFolder(cur_path)
if trash:
try:
moveServiceFiles(current, trash, name, allowCopy=True)
self["list"].removeService(current)
self.showActionFeedback(_("Deleted") + " " + name)
return
except:
msg = _("Cannot move to the trash can") + "\n"
are_you_sure = _("Do you really want to delete %s ?") % name
else:
msg = _("Cannot move to the trash can") + "\n"
are_you_sure = _("Do you really want to delete %s ?") % name
for fn in os.listdir(cur_path):
if (fn != '.') and (fn != '..'):
ffn = os.path.join(cur_path, fn)
if os.path.isdir(ffn):
subdirs += 1
else:
if ffn.endswith(".ts"): # New condition to only count recordings rather than number of files (can be up to six files per recording, I think)
files += 1
if files or subdirs:
folder_filename = os.path.split(os.path.split(name)[0])[1]
# Was mbox=self.session.openWithCallback(self.delete, MessageBox, _("'%s' contains %d file(s) and %d sub-directories.\n") % (folder_filename,files,subdirs) + are_you_sure)
# Replacement line to reflect it is now a recording rather than file count:
mbox=self.session.openWithCallback(self.delete, MessageBox, _("'%s' contains %d recording(s) and %d sub-directories.\n") % (folder_filename,files,subdirs) + are_you_sure)
mbox.setTitle(self.getTitle())
return
# New code to delete empty directories without an "are you sure" question
else:
try:
os.rmdir(cur_path)
except Exception, e:
print "[MovieSelection] Failed delete", e
self.session.open(MessageBox, _("Delete failed!") + "\n" + str(e), MessageBox.TYPE_ERROR)
else:
self["list"].removeService(current)
self.showActionFeedback(_("Deleted") + " " + name)
from Screens.InfoBarGenerics import delResumePoint
delResumePoint(current)
return
# End of new code
else: # Deleting from trash can or no trash can "installed"
if '.Trash' in cur_path:
etc
etc

appears to do what I want, though I have only given it minimal testing. I have left the replaced code in place, but commented it out, to make the changes more obvious.

birdman
06-04-16, 16:08
However my original point stands - to me wrongly deleting an empty folder ...Deleting any folder can (IIRC) result in you having Locations/Bookmarks which do not exist.

EMJB
06-04-16, 16:47
Deleting any folder can (IIRC) result in you having Locations/Bookmarks which do not exist.I have yet to find a problem with the folders I have deleted, but will keep an eye on the situation. If you remember what they are, I would be interested to hear, but such a situation seems very unsatisfactory to me. On my Topfield PVR, I guess I deleted a folder on average every couple of weeks, so if I can't delete them here things are going to get very cluttered by the end of the year!

If significant problems can exist from deleting a folder other than the loss of any recordings therein, shouldn't the "are you sure" message at least give a clear indication of what they may be?

ccs
06-04-16, 16:55
The way I am using my machine, all folders originate from auto-timers, and would get re-created if erroneously deleted.I wondered about this when you posted earlier.
When I altered an autotimer to no longer save to a folder, I changed the autotimer first, before deleting the folder.
I didn't want to risk autotimers failing because a bookmark pointed to a non existent folder.

EMJB
06-04-16, 17:14
@ccs - you have me worried now. I have an autotimer with defined destination due this evening, so have renamed that existing directory so will see whether I was right in assuming it would be recreated as I thought would be the case.

birdman
06-04-16, 18:24
@ccs - you have me worried now. I have an autotimer with defined destination due this evening, so have renamed that existing directory so will see whether I was right in assuming it would be recreated as I thought would be the case.But bear in mind you can only set the Autotimer to put the recording into a directory if that directory is on the Locations/Bookmarks list which means that (barring hand-editing of the settings file) it will have had to exist at some point in the past

EMJB
06-04-16, 18:54
But bear in mind you can only set the Autotimer to put the recording into a directory if that directory is on the Locations/Bookmarks list which means that (barring hand-editing of the settings file) it will have had to exist at some point in the past which is a limitation I found very frustrating after having an Auto-File option on the Topfield which puts series recordings in a folder derived from the programme name. However is this a issue in the case where we are potentially deleting the folder in error?

EMJB
07-04-16, 09:19
I have an auto-timer set up to record Horizon to a sub-folder called "Horizon". Last night I changed the name of the existing folder to "HorizonWas" using Samba, and the new episode was recorded to a new directory called "Horizon".

Are, I can hear you saying, perhaps that was because it had some memory of the old folder that might be destroyed if you deleted it in the normal way, so I set up at new autotimer to record to a non-existent, and never previously existed, folder. This was duly created.

I therefore reach the conclusion that there is NO penalty in deleting an empty folder, and so there should be no "are you sure" message. If I am wrong, then surely the "are you sure" message needs to indicate that penalty as clearly the vast majority of users will just go ahead and delete the folder in ignorance of what penalty they will incur.