Rockbox

Tasklist

FS#7486 - Favourites plugin.

Attached to Project: Rockbox
Opened by Bryan Childs (GodEater) - Thursday, 26 July 2007, 15:03 GMT
Last edited by Jonathan Gordon (jdgordon) - Sunday, 05 August 2007, 12:14 GMT
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 2
Private No

Details

This is a first effort at a "favourites" plugin, which will allow the user to store up to ten shortcuts to directories somewhere in their file tree.

TODO:
Add functionality to write the .rockbox/rocks/favourites.link file since this currently needs to already exist with entries in it for the plugin to do much. The format of the file is assumed to be one directory path per line, e.g. :

/dir1/
/dir2/dir3/

The trailing slash is a must have.

BUGS:

Currently the call to set_current_file(), which is supposed to move the file browser to the selected directory is causing the sim to seg fault. Haven't even tried this on an actual DAP yet.
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Sunday, 05 August 2007, 12:14 GMT
Reason for closing:  Accepted
Comment by Bryan Childs (GodEater) - Thursday, 26 July 2007, 15:07 GMT
And here's the patch...
Comment by Bryan Childs (GodEater) - Thursday, 26 July 2007, 15:24 GMT
Fixed the segfault, but the plugin isn't displaying much after the set_current_file() call now.
Comment by Bryan Childs (GodEater) - Friday, 27 July 2007, 11:08 GMT
With a bit more hackery, the plugin now jumps to the right place in the filetree even when launched from the plugin menu.

This means hacking the dirfilter setting on the plugin browser, but I don't think anyone will notice ;)
Comment by Bryan Childs (GodEater) - Friday, 27 July 2007, 12:18 GMT
Tidied the code up a lot, and split a lot of functions up to make them more re-use friendly.

TODO:

1) Still need the ability to write the favourites file itself.
2) Possibly add directly to the context menu for directories to make adding entries quick

BUGS:

None that I know of ;)
Comment by Bryan Childs (GodEater) - Friday, 27 July 2007, 16:07 GMT
Bit more reorganisation.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 11:14 GMT
This now features the writing of the favourites file, and a context menu entry for it when the user wants to add a directory.

Still TODO:
Allow user to remove entries from the favourites file.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 11:24 GMT
Removed a call to rb->gui_synclist_draw() at JdGordon's behest.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 11:33 GMT
Whoops - managed to leave a couple of calls to glibc's snprintf() instead of rb->snprintf() in the code.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 14:27 GMT
Another whoops - I managed to have "Add to favourites" in the context menu even if the selected item wasn't a directory. This fixes that.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 15:03 GMT
Make the plugin display an error if the directory selected in the favourites file no longer exists on disk.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 15:22 GMT
Tidied up some function prototypes to get rid of warnings when building for an actual DAP rather than just as a simulator.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 19:28 GMT
Moved the location of the favourites.link file to the root dir (don't panic amiconn, it's only created if you use the plugin).

Fixed a bug with saving a new file.
Comment by Bryan Childs (GodEater) - Monday, 30 July 2007, 19:53 GMT
Oops. Just noticed I missed out a load of things from the patch. Like most of it's functionality.
Comment by Gerritt Gonzales (GRaTT) - Monday, 30 July 2007, 20:47 GMT
I am using a sansa and it will not work for me.
Error is "directory no longer exist on disk"
for all links made by the plugin.
If I edit the link file and remove the end /
the plugin exits to the file browser with the
favourite link selected.
In other words one directory up from the link
with the link directory selected.

I would like to see this selectable from the main menu
and be able to start plugins as well or songs.
GRaTT
Comment by Jeton Aliji (jeton) - Monday, 30 July 2007, 22:45 GMT
I would suggest you include a template file/config in the patch so that it doesn't cause problems with users trying to make it work. And from that template they could easily change the path of the folders.
Comment by Bryan Childs (GodEater) - Tuesday, 31 July 2007, 06:50 GMT
@GRaTT:

I'm willing to bet you created the .link file on your computer and copied it to the sansa correct ?

@jeton

No need to include a template - the plugin creates the file and adds the entries to it itself. Having a template file will only lead people to editing it in a windows text editor and breaking the format.
Comment by Bryan Childs (GodEater) - Tuesday, 31 July 2007, 13:57 GMT
Firstly, sorry for the size of the patch - my diff seems to think I've changed masses of stuff in onplay.c - which I haven't.

Secondly - this has now been completely reworked to use the rb->read_line() function instead of my own file parsing code - so windows edited files shouldn't break the functionality anymore.

Thirdly - you can now mark individual files as well as directories.

Fourthly - I've renamed the patch to "shortcuts" to avoid confusion with an earlier "favourites" plugin which was ditched months and months back.
Comment by Bryan Childs (GodEater) - Tuesday, 31 July 2007, 14:17 GMT
Be an idea to actuall rename the patch too I suppose ;)
Comment by Gerritt Gonzales (GRaTT) - Tuesday, 31 July 2007, 14:58 GMT
@Godeater
I only used the link file as created on the sansa.
Also edited it on the sansa to test it by removing the end slash(/).
I will try the new patch and recreate the link file.
GRaTT
Comment by Jeton Aliji (jeton) - Tuesday, 31 July 2007, 16:40 GMT
I'm using cpchan's build on Sansa. When i start the plugin i get "Couldn't find the file".
Comment by Bryan Childs (GodEater) - Tuesday, 31 July 2007, 19:38 GMT
@jeton : If you've compiled this plugin into your build, then you're not using cpchan's build anymore...
Comment by Bryan Childs (GodEater) - Wednesday, 01 August 2007, 12:46 GMT
First attempt at adding a "delete" function. Which sadly doesn't work at the moment. No idea why though.
Comment by Bryan Childs (GodEater) - Wednesday, 01 August 2007, 15:21 GMT
Now with new working delete option!

In order to delete an entry from the links file, click on it with the "Menu" key.

**WARNING* There is no prompt currently to check if you're sure of not - it just gets wiped.
Comment by Bryan Childs (GodEater) - Wednesday, 01 August 2007, 19:50 GMT
After many requests, I've relented.

This patch now handles user created .link files too (you have to create them in some sort of text editor).

1) If you wish your shortcut to enter a directory, make sure the entry has a trailing / on it. If it doesn't, the plugin will "select" the directory in it's parent, rather than entering it.

2) If your user created .link file has only one entry, the plugin will take you straight there rather than displaying the menu.

3) I've disallowed deleting entries in user created .link files. At least for the time being.
Comment by Gerritt Gonzales (GRaTT) - Wednesday, 01 August 2007, 20:45 GMT
It still does not work for directories for me.
I have to remove the trailing slash (/) and then it will
go to the directory and highlight it not enter it.
As is it will display ""Directory no longer exists on disk"

The single link does not work for me either.
Thanks for relenting.
GRaTT
Comment by Gerritt Gonzales (GRaTT) - Wednesday, 01 August 2007, 21:09 GMT
Some additional info.
If I run the plugin from the plugins menu
and select my fixed link tailing slash removed,
or a file link, the plugin just drops me in the
directory and does not highlight the link file/dir.
If run from the link file it highlights the file/dir.
GRaTT
Comment by Bryan Childs (GodEater) - Thursday, 02 August 2007, 10:23 GMT
Fixed a few issues I found with handling user defined link files.

@GRaTT:

Could you attach some of your .link files for me to look at please ?
Comment by Gerritt Gonzales (GRaTT) - Thursday, 02 August 2007, 14:36 GMT
Here is a fresh made link file. Made by the plugin.
I have not yet updated to the last patch, and
I do have some other patches in my build.
If there is a trailing slash in the link file
an error as above "no longer exists"
GRaTT
Comment by Bryan Childs (GodEater) - Thursday, 02 August 2007, 14:46 GMT
Please update to the latest patch version and let me know if that sorts the problem out first.
Thanks for the file.
Comment by Bryan Childs (GodEater) - Thursday, 02 August 2007, 14:49 GMT
GRaTT, do none of these entries work?

I have no idea if the <microSD1> entries will work or not as they're "fake" file system entries, but the first two look like they should work to me.
---
/MUSIC/
/RECORD/VOICE/
/<microSD1>/GRaTT/slideshow/
/<microSD1>/movies/movies/
/<microSD1>/music/Shakira/Laundry Service/Eyes Like Yours (Ojos Asi).mp3
---
Comment by Gerritt Gonzales (GRaTT) - Thursday, 02 August 2007, 15:09 GMT
GodEater
Only the one that points to a file works.
The others will work if I remove the trailing slash,
but only enter the dir above and highlight the link dir.
The <microSD1> "fake" file system entries act the same way so they do "work".
It did work originally for me so it may be one of the other patches.
I just svn uped and will try with just this patch.
GRaTT
Comment by Gerritt Gonzales (GRaTT) - Thursday, 02 August 2007, 15:40 GMT
Just tried and all the same symtoms as earlier posts.
Including different behavior between running the link file
and the plugin. Also single line links go to the list.
info: sansa E260 with 2GB SD card
OF Version ...18A
RB ver. r14147M-070802
shorcuts ver. Aug 02 10:23
The version I think worked. The comment was "Bit more reorganisation.
I will verify this in the next few days.
GRaTT
Comment by Bryan Childs (GodEater) - Thursday, 02 August 2007, 18:12 GMT
That's peculiar because I haven't touched the actual "change place in the file tree" code since the second revision.
Comment by Bryan Childs (GodEater) - Friday, 03 August 2007, 08:48 GMT
Re-organised the code a bit to get rid of a lot of the if/else nesting. Hopefully this makes it a bit easier to follow.

I've also removed the shortcuts.h file and put its content into shortcuts.c - linuxstb suggested this since the shortcuts code is currently unreferenced by any other part of the RB code - and so the .h file isn't really required.

Have so far not done anything about his suggestion to move away from a linked list to a static buffer. That's just too much re-writing for now!
Comment by Bryan Childs (GodEater) - Friday, 03 August 2007, 10:08 GMT
Helps if you remove the #include too...
Comment by Bryan Childs (GodEater) - Friday, 03 August 2007, 10:57 GMT
This should fix the issue with non-dircache targets (Sansa) thinking files/directories are missing when they're not!
Comment by Bryan Childs (GodEater) - Friday, 03 August 2007, 11:08 GMT
sync'd with svn.
Comment by Bryan Childs (GodEater) - Friday, 03 August 2007, 15:31 GMT
Sync'd again.
Comment by Bryan Childs (GodEater) - Friday, 03 August 2007, 15:36 GMT
Oops - shortcuts.h resurrected itself!
Comment by Gerritt Gonzales (GRaTT) - Friday, 03 August 2007, 16:58 GMT
The latest version works for me as expected with directories.
The above discrepancies between running the link file
or running the plugin and the file/dir getting selected/highlighted still apply.
The single link still does not work.
GRaTT
Comment by Gerritt Gonzales (GRaTT) - Saturday, 04 August 2007, 20:29 GMT
My error, single links works as expected,
only on user defined link files.
GRaTT
Comment by Bryan Childs (GodEater) - Saturday, 04 August 2007, 20:44 GMT
Correct - that's as designed. Only user created links allow the "single entry" feature - since they have to be manually edited.

The "shortcuts.link" file however can't work like that, or the user is unable to delete the entry from it.
Comment by Gerritt Gonzales (GRaTT) - Sunday, 05 August 2007, 08:42 GMT
last bit of info:
I only get the non highlight behavior when run from the plugins menu.
I tried running the shorcuts.rock file from the directory view and
both the directory without a / and the file were highlighted as expected.
GRaTT
Comment by Bryan Childs (GodEater) - Sunday, 05 August 2007, 09:14 GMT
Launching the plugin from "directory" view (i.e. the file browser) and the plugin's menu is the same thing. The plugins menu is simply the file browser with a filter applied to it so you can only see .rock files.
Comment by Bryan Childs (GodEater) - Sunday, 05 August 2007, 09:28 GMT
Latest version sync'd against svn.
Comment by Bryan Childs (GodEater) - Sunday, 05 August 2007, 09:37 GMT
Sync'd again
Comment by Bryan Childs (GodEater) - Sunday, 05 August 2007, 09:39 GMT
Note to self - make sure you've commited to git before doing the diff, and also check the size of your patch file!

Loading...