Interesting, WeeWX-WD installs 4 skins and I have never had a case where 
--uninstall has not removed them all. Pushed for time this morning but I 
will sit down and have a play with this this arvo. Pat, I presume the just 
use your repo as per the latest commit?

Gary

On Thursday, 6 September 2018 03:54:27 UTC+10, Pat O'Brien wrote:
>
> I'm working on an update to my Belchertown skin which will really add 2 
> skins to SKIN_ROOT. One skin is the HTML files, and the other is a 
> highcharts fork. Certain users are having a high latency running the skin 
> as it is today, so splitting into 2 offers a faster generation time since 
> highcharts has a lot database queries, and a separate skin offers less SLE 
> loops.  
>
> During my testing I found that the --uninstall function does not remove 
> multiple SKIN_ROOT directories that the install.py placed there. 
>
> I found in weecfg/extensions.py, the uninstaller command is only removing 
> those directories with the most common root - which does not apply to all 
> SKIN_ROOT directories since there are multiple roots. 
>
> I made a small change to the extensions.py (diff below) which handles the 
> multiple root directory removal. Instead of finding the commonprefix, I 
> just loop through the directory_list, then loop through each item for 
> removal. 
>
> root@setuppy:~# diff extension.py extension_updated.py
> 402,407c402,407
> <         # Start by finding the directory closest to root
> <         most_root = os.path.commonprefix(directory_list)
> <         # Now delete the directories under it, from the bottom up.
> <         for dirpath, _, _ in os.walk(most_root, topdown=False):
> <             if dirpath in directory_list:
> <                 self.delete_directory(dirpath)
> ---
> >         for dir in directory_list:
> >             # Loop through each directory listing
> >             for dirpath, _, _ in os.walk(dir, topdown=False):
> >                 # Delete the directory
> >                 if dirpath in directory_list:
> >                     self.delete_directory(dirpath)
>
>
> I wanted to raise it here to see if this is a valid change and discuss. 
> Happy to create an issue and PR if so. 
>

Reply via email to