Rockbox

Tasklist

FS#10187 - Theme Deleter - Parses CFG files and removes their files

Attached to Project: Rockbox
Opened by Mattitiah Curtis (motionman95) - Sunday, 03 May 2009, 22:13 GMT
Last edited by Teruaki Kawashima (teru) - Monday, 14 December 2009, 05:51 GMT
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This is plugin that will parse a given config (.cfg) file and remove it's files in an attempt to "remove" the files installed by a theme, as requested by BdN3504 (on the rockbox forums). This is the plugin in it's most basic/beta state, TONS of work must be done before it's complete.

For now it relies on the "cabbiev2.cfg" file being placed in the same directory as it. Also, as pointed out in the forums (I forget by who) that it may not be a good idea to remove certain files, such as the font or iconset files. So, to sum it all up, this plugin is really just a starting point, and user input will definitely be a factor in it's growth.
This task depends upon

Closed by  Teruaki Kawashima (teru)
Monday, 14 December 2009, 05:51 GMT
Reason for closing:  Accepted
Additional comments about closing:  committed in r23130.
Comment by Hilton Shumway (HIllshum) - Tuesday, 05 May 2009, 16:28 GMT
The entries in CATEGORIES and SOURCES should follow alphabetical order, and the font doesn't come as part of the theme and should not be deleted
Comment by Dominik Riebeling (bluebrother) - Tuesday, 05 May 2009, 19:00 GMT
Your patch doesn't follow the coding style guidelines (spaces instead of tabs)

Hilton: there are several themes that *do* include a font. This is nothing special and has allways been that way -- the only exception are the themes shipped with the rockbox builds and themes relying on the fonts package. The more important thing is that before deleting a file all other themes need to get checked if some other theme is using that file. Especially fonts, but other files might be affected as well. Also, I don't see a reason thy CATEGORIES and SOURCES would need to be alphabetical -- in fact, SOURCES is definitely *not* sorted alphabetically. Please don't make such obvious wrong statements as that will only cause confusion.
Comment by Hilton Shumway (HIllshum) - Tuesday, 05 May 2009, 23:58 GMT
@Dominik: They are alphabetical anyway.
Comment by Dominik Riebeling (bluebrother) - Thursday, 07 May 2009, 19:56 GMT
Hilton: a somewhat random snippet from apps/plugins/SOURCES:

#ifdef HAVE_LCD_BITMAP /* Not for the Player */
rockblox1d.c
brickmania.c
maze.c
mazezam.c
text_editor.c
wavview.c
robotfindskitten.c

from my understanding of "alphabetical" brickmania.c has to come before rockblox1d.c if that would be sorted. Thus, they are NOT sorted alphabetical. Parts of the file are, but for compiling it doesn't matter at all. You call that alphabetical?
Comment by David Kauffmann (BdN3504) - Thursday, 14 May 2009, 16:24 GMT
can't you use what i already wrote on the forum, to get the locations?
[...] in settings.h

#define WPS_DIR ROCKBOX_DIR "/wps"
#define THEME_DIR ROCKBOX_DIR "/themes"
Comment by Mattitiah Curtis (motionman95) - Friday, 15 May 2009, 15:27 GMT
Yeah, but I'm not sure if there would be any point - seeing that it's loaded from the CFG file...
Comment by Teruaki Kawashima (teru) - Saturday, 23 May 2009, 12:19 GMT
i wrote another plugin to remove theme.
this works as viewer and user can select to remove a file or not.
only tested on a simulator.
Comment by David Kauffmann (BdN3504) - Sunday, 24 May 2009, 15:15 GMT
i compiled your patch with a check out from yesterday, but it seems corrupted. First there is no version info file in the zip file i created, then i had to execute "make zip" two times, because "make && make zip" did not give me a zip output file. The patched build runs well, but as i understood your approach, you made it as a viewer plugin for cfg files, right? but when i enter the context menu of a cfg file and select "delete theme", only the cfg file is removed. another strange thing is, that when i tried to open a .txt file, it did not show up like with the daily build, but three splash messages are displayed on top of each other, the last one (which is the only one that is readable) says "done". when i then enter the context menu for txt files, and choose "open with" i get only three options: search, sort, properties.
Something must have gone wrong, although i get no errors, when i compile.
Comment by Teruaki Kawashima (teru) - Monday, 25 May 2009, 13:48 GMT
Maybe something was wrong. Could you try this one?
I tested both simulator and my gigabeat. version is r21072.
I forget to write how to use this plugin, sorry.
usage:
goto "Main Menu" -> "Settings" -> "Theme Settings" -> "Browse Themes".
enter Context Menu for the cfg file to remove, and execute theme_remove in "Open With...".
selecting "Remove Theme" will remove wps, backdrop, and the cfg file. Font is not removed by default.
Comment by Gman (Thecoolgman) - Tuesday, 26 May 2009, 09:11 GMT
It works great for me
Comment by David Kauffmann (BdN3504) - Tuesday, 26 May 2009, 20:23 GMT
i compiled with r21090 and it works. I think you did it very nice. Including the theme remover in the cfg file context menu would mean you'd have to code it into the core, right? or would it be easily possible? that's the only thing i'd like to improve. You did a really good job, thank you very much!
Comment by Mattitiah Curtis (motionman95) - Wednesday, 27 May 2009, 00:25 GMT
Thanks for the viewer, teru. I'd been really busy of late and had forgotten about this patch. But it seems that you've done a much better job! Glad to see that my work inspired you, if it did.
Comment by Teruaki Kawashima (teru) - Wednesday, 27 May 2009, 13:01 GMT
Thanks for the feedbacks. I'm glad to hear it worked.
I think it's not so difficult to add "Remove Theme" option to the context menu.
IIUC, "Properties" and "Add to Shortcuts" in the context menu use plugins, so i guess it can be done in the same way.
Comment by Gman (Thecoolgman) - Thursday, 16 July 2009, 01:33 GMT
Needs resync
Comment by Teruaki Kawashima (teru) - Tuesday, 21 July 2009, 14:30 GMT
sync and update patch.
*add option "Remove if not Used".
*save options.
*create log file "/theme_remove_log.txt".

also, attach a patch to add item to context menu.
for this, apply theme_remove_context_menu.patch with theme_remove.patch.
open context menu on .cfg file and "Remove Theme" will be shown in it.
selecting this is same as selecting theme_remove in "Open With...".
Comment by David Kauffmann (BdN3504) - Tuesday, 21 July 2009, 17:50 GMT
This is fantastic!
i have compiled a sim with both patches and uploaded it to my rs account. if anybody wishes to see how this feature looks on target check it out!

http://rapidshare.com/files/258412449/PatchedGigaSimrar.zip

i hope this patch will make it into svn, cross your fingers!
Comment by Teruaki Kawashima (teru) - Friday, 24 July 2009, 13:29 GMT
I think manual is needed to commit this patch...
Comment by David Kauffmann (BdN3504) - Sunday, 26 July 2009, 11:05 GMT
OK.

One glitch i have observed: In the log file, if an item is not present it says: "Not exists". This shall be "Does not exist".

A suggestion for improvement: Attach some ifdefs for targets that don't have remote control displays. For example i think it's unneccessary to ask a gigabeat owner to remove the "remote wps" or the "remote viewers iconset" or the "remote iconset" because these things don't exist on targets without a remote display.
Comment by David Kauffmann (BdN3504) - Sunday, 26 July 2009, 13:04 GMT
My first attempt was a bit shitty. Here's the proper version which also includes some fixes for that part of the manual.
Comment by David Kauffmann (BdN3504) - Sunday, 26 July 2009, 13:40 GMT
final version, fixed two minor glitches.
Comment by Teruaki Kawashima (teru) - Sunday, 26 July 2009, 15:28 GMT
nice work. thank you!
i turned it to plugin manual and added ifdefs.
also, integrate almost same option descriptions.
Comment by David Kauffmann (BdN3504) - Monday, 27 July 2009, 09:17 GMT
I have done some minor corrections. Although i like the idea of having the plugin in the context menu like with your theme_remove_context_menu.patch much better. do you think it's more feasible to implement it as a plugin?
Comment by Dave Chapman (linuxstb) - Monday, 27 July 2009, 13:34 GMT
The manual entry says "The changes done to the files could break another theme." - can you expand on this?

I can think of the case where fonts from rockbox-fonts.zip are deleted by this plugin, and then themes installed at a later date which rely on that theme will stop working. Is there anything else?

I'm just slightly worried that if it's not perfect, then this plugin could do more harm than good.
Comment by Teruaki Kawashima (teru) - Monday, 27 July 2009, 16:14 GMT
improve a bit. now, there is no problem I know.
*use global_settings struct to check files currently used.
*add rockbox-fonts.config which contains filenames in rockbox-fonts.zip to prevent removing fnt files from the zip.

imo it would be possible to add context menu after plugin is committed, although i'm not sure if it is worth doing or safe enough.
Comment by David Kauffmann (BdN3504) - Monday, 27 July 2009, 22:21 GMT
One last thing:

Could you change the title of the menu from "Theme Remove" to "Remove [Theme Name]" ? (and maybe let it scroll, if the theme's name is too long)

i attached a proper manual patch containing the directory locations.
Comment by Dave Chapman (linuxstb) - Tuesday, 28 July 2009, 06:20 GMT
Is the note in the manual still needed (about possibly breaking themes) ? According to Teruaki's last post, there are no known issues.
Comment by David Kauffmann (BdN3504) - Tuesday, 28 July 2009, 12:27 GMT
I think if you choose always remove, then there is the possibility that other themes don't work. but i am not sure. if that'S not the case, then we take that note out of the manual of course.
Comment by Mattitiah Curtis (motionman95) - Tuesday, 28 July 2009, 19:45 GMT
Shouldn't this be moved to a new patch? I mean, it isn't my patch anymore...at least not really.
Comment by David Kauffmann (BdN3504) - Tuesday, 28 July 2009, 20:51 GMT
i don't understand... everything that has been posted here serves the purpose of the plugin mentioned in the task title. So what's the significance of the name of the person who started this patch? i think there's no need to move it to a new task. which purpose would that serve?
On topic: apparently i am too noob to post a proper patch. The very first line shall of course be

Index: manual/plugins/theme_remove.tex

and not

Index: theme_remove.tex

sorry, but i am still learning. plus i did svn add theme_remove.tex and then svn diff, because i thought that's how it works, but apparently either it just doesn't or i did something wrong.
Comment by Teruaki Kawashima (teru) - Wednesday, 29 July 2009, 15:49 GMT
*change title of menu from "Theme Remove" to "Remove [themename]".

I believe this plugin basically wont cause problem. but this plugin isn't perfect and I have no idea what kind of things would happen.
maybe what we need is more tester?

BdN3504: maybe you added theme_remove.tex in wrong directory?
Comment by David Kauffmann (BdN3504) - Wednesday, 29 July 2009, 23:05 GMT
brilliant! i tested and it even scrolls! nice!

yes, i added it in the source... now i used svn diff correctly and have it right patch for the right files.
thanks to pixelma, i now know how to make a proper newline. before the note i could remove the descriiption tags in the first paragraph.
Comment by David Kauffmann (BdN3504) - Saturday, 01 August 2009, 02:43 GMT
i just finished compiling patched versions of rb for all currently supported targets (see froums). what do you want to have tested exactly?
Comment by JerryLange (psycho_maniac) - Monday, 10 August 2009, 02:07 GMT
Player: Gigabeat F40
Bug: Go to "Menu/Settings/Theme Settings/While Playing Screen/" now go back to "Theme Settings"
Then to go to /Browse Theme Files/" it says "Says "No Files!"
but if you exit the menu and got to /Browse Theme Files/ FIRST it shows the files that are there.
Comment by JerryLange (psycho_maniac) - Monday, 10 August 2009, 02:09 GMT
cant edit previous comments???

*add "I am running Running Version: r22090M-090801"
Comment by Teruaki Kawashima (teru) - Monday, 10 August 2009, 13:26 GMT
I think that is not a bug of the patch and is fixed in r22094.
Comment by JerryLange (psycho_maniac) - Monday, 10 August 2009, 17:11 GMT
alirght. guess ill wait for a new version on the forums. thats where i got the one im testing with
Comment by David Kauffmann (BdN3504) - Thursday, 13 August 2009, 16:14 GMT
OK, so it was a dumb idea to patch a current unstable build. i have learned from my mistakes and have created a patch suitable for release 3.3.
The only thing i had to change were the strlcpy s to strncpy s. i have tested it on a sim and it doesn't cause a problem, even if you have extremely long filenames.
So for testing purposes, i compiled five 3.3 versions and posted them to the forums. This time, i only compiled for targets which have actually been downloaded. if you want a testbuild for a specific target which isn't in the forumthread, either contact me, or download and apply the patch i provide after you check out 3.3 using

svn co svn://svn.rockbox.org/rockbox/branches/v3_3 rockbox-3.3
Comment by David Kauffmann (BdN3504) - Saturday, 15 August 2009, 21:03 GMT
The patch is still up to date. i had to resync the manual entry though. and psycho_maniac didn't quite understand how the plugin works, so i added a reference to the entry.
Comment by JerryLange (psycho_maniac) - Wednesday, 19 August 2009, 23:26 GMT
would be be able to just hold select and get "delete theme" or would this be the only way of doing it this way?
Comment by David Kauffmann (BdN3504) - Wednesday, 19 August 2009, 23:29 GMT
if you use http://www.rockbox.org/tracker/task/10187?getfile=20104 posted http://www.rockbox.org/tracker/task/10187#comment31722 you'll have it in the context menu, which i think is more convenient too.
Comment by JerryLange (psycho_maniac) - Thursday, 20 August 2009, 01:28 GMT
would be be able to just hold select and get "delete theme" or would this be the only way of doing it this way?
Comment by David Kauffmann (BdN3504) - Thursday, 20 August 2009, 18:29 GMT
well now, last today i wrote that exactly. if you no know what i mean, no there is no other way of doing this.
you don't know how compile?
you have vmwareplayer installed? can use vmware image debian.7z provided in wiki?
if yes login root. name: root password: rockbox
hit windows-e.
assume you winxp: go My Network Places.
if root at samba is shown double click
if not type \\debian\root into adress toolbar in current window and press enter
wait
asked for login again root/rockbox
save this page to \\debian\root name file: td.patch
http://www.rockbox.org/tracker/task/10187?getfile=20267
save this page to same dir name file: cm.patch
http://www.rockbox.org/tracker/task/10187?getfile=20104
click on vmware
right click open shells -> eterm
execute command: vmware-toolbox& (note: sic!)
now you can copy text into bash. use ctrl-c and paste into shell by using shift+insert.
execute following command (assuming your player is gigabeat f):
cd && svn co svn://svn.rockbox.org/rockbox/branches/v3_3 rockbox && cd && patch -p0 < td.patch && patch -p0 < cm.patch && cd ~/rockbox && mkdir giga && cd giga && ../tools/configure --target=gigabeatf --type=n && make && make zip && mv rockbox.zip ~/gigabeat-f-patched-33.zip && rm -rf ~/rockbox

this command make you build with patch applied to 3.3 and context menu.
Comment by JerryLange (psycho_maniac) - Sunday, 23 August 2009, 22:49 GMT
wow aparently i didnt see your previous comment. sorry
Comment by David Kauffmann (BdN3504) - Sunday, 30 August 2009, 19:42 GMT
will this be committed some time soon?
the test builds have been up since about two weeks and there hasn't been a single bug iiuc. the manual is ready and the functionality is given.
Comment by JerryLange (psycho_maniac) - Friday, 04 September 2009, 15:45 GMT
i think it would be simpler if it was in the context menu
i know you can patch it so its like that but will the final version be in the context menu? like theme_remove.3.patch
Comment by David Kauffmann (BdN3504) - Thursday, 24 September 2009, 22:08 GMT
Sync, only offset errors. Second patch has context menu already included.
Comment by David Kauffmann (BdN3504) - Thursday, 24 September 2009, 22:31 GMT
something went wrong with the buildzip.pl. don't know what, but now it's fixed.
Comment by Teruaki Kawashima (teru) - Thursday, 08 October 2009, 12:45 GMT
Update patch.
Comment by Teruaki Kawashima (teru) - Sunday, 11 October 2009, 14:12 GMT
Update patch.
* change string "Ask" to "Ask for Removal".
* set default setting of font to "Ask for Removal".
* Use splash function to show message.
* add some comments.
Comment by Teruaki Kawashima (teru) - Monday, 12 October 2009, 14:55 GMT
update patch to add item to context menu.
Comment by David Kauffmann (BdN3504) - Sunday, 13 December 2009, 16:10 GMT
TTL has expired on this task. see http://svn.rockbox.org/viewvc.cgi?view=rev;revision=23130 . please close it, so it won't get listed in the "open bugs" section on the ml any more.

Loading...