Rockbox

Tasklist

FS#7667 - Split shortcuts plugin into a viewer and an appender

Attached to Project: Rockbox
Opened by Alexander Levin (fml2) - Monday, 27 August 2007, 20:37 GMT
Last edited by Peter D'Hoye (petur) - Monday, 03 September 2007, 22:25 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 0
Private No

Details

This simplifies code, we don't have to analyse whether it's a .link file etc -- each part has its own little task.

Also, two options are introduced:

1. In the link file, you can specify the display name for the entry. It should be separated from the path by the TAB. E.g.

/my/path/<TAB>My favourite songs

2. In the file, you can write

#Display last path segments=N

and then only the last N path segments will be displayed. This is handy if your music is in a deep folder.

Explicit name (with a TAB) has higher priority than N, i.e. if N=2 and you have

/aa/bb/cc/x.mp3<TAB>YYY

then the item will be displayed as YYY and not as bb/cc/x.mp3


I get some strange linker errors. Before I split the plugin into two everything worked OK.
This task depends upon

Closed by  Peter D'Hoye (petur)
Monday, 03 September 2007, 22:25 GMT
Reason for closing:  Accepted
Additional comments about closing:  I did find two tabs and a too long line :p
Comment by Alexander Levin (fml2) - Tuesday, 28 August 2007, 15:26 GMT
Please somebody help me with the makefile. There are three source .c files in plugins/shortcuts: shortcuts_common.c, shortcuts_view.c, and shortcuts_append.c (and one .h file)

Out of these three files, two .rocks should be produced:
1. shortcuts_common.c + shortcuts_view.c -> shortcuts_view.rock
2. shortcuts_common.c + shortcuts_append.c -> shortcuts_append.rock

How can this be done?

I somehow managed to produce a patch file with duplicated patches. Attached is the corrected patch.
Comment by Alexander Levin (fml2) - Tuesday, 28 August 2007, 22:19 GMT
Now everything gets compiled and linked (many thanks to linuxstb). Viewer works. But the appender doesn't get loaded. On the sim, I get error message "Incompatible version". And have no idea about the reason. Linking problem?
Comment by Alexander Levin (fml2) - Wednesday, 29 August 2007, 18:56 GMT
Now everything works. Thanks go to linuxstb again for spotting the problem in shortcuts_append.c. This could be committed if you ask me. Feedback is welcome.
Comment by Peter D'Hoye (petur) - Thursday, 30 August 2007, 19:25 GMT
I can understand most of your changes, except splitting the plugin in two... It wasn't that big and complicated, was it?
Comment by Alexander Levin (fml2) - Thursday, 30 August 2007, 20:08 GMT
Now everything works as in the original, i.e. selecting a file entry places the cursor at the file in the file browser. Selecting a dir entry enters that dir and places the cursor at the first file inside it.

Since I was one of the initiators for the creation of the plugin and am one of its very few users (if not the only one :-) could you please commit it asap.

@petur: after the splitting, each part has its own little but well defined task. Before splitting, if you selected a .link file and wanted to add it to the shortcuts, it was shown instead -> not the principle of the least surprise IMHO. Another thing was that the association of the plugin with .link files was hard coded in the plugin. IMHO it should be done in viewers.config only.
Comment by Alexander Levin (fml2) - Thursday, 30 August 2007, 20:34 GMT
Corrected patch. Now we correctly determine if it's a file or a dir (sim and target behave differently when calling open()).
Comment by Peter D'Hoye (petur) - Thursday, 30 August 2007, 21:22 GMT
some remarks:
1) the opening braces of a function should be on a new line (this is not yet in CONTRIBUTING but it will be soon)
2) I do not understand how the shortcuts to shortcuts works. With your patch you can add it but afterwards it doesn't look like you can do much with it (I thought it would open the shortcut file)
Comment by Alexander Levin (fml2) - Friday, 31 August 2007, 06:58 GMT
to 1) I'll correct this (are there tools to do that automatically?)

to 2) Shortcuts are just a way to quickly go to a certaint dir/file. It doesn't dictate what you should do with it then, just takes you there in the file browser. Just a shortcut. It doesn't "play" the file. So you could put a reference to a link file into another link file and quickly jump from dir to dir :-)
Comment by Peter D'Hoye (petur) - Friday, 31 August 2007, 18:06 GMT
there were not that many {} violations, I did it here in order to test and it took < 5m ;)

thanks for clarifying 2)
Comment by Alexander Levin (fml2) - Friday, 31 August 2007, 21:49 GMT
Some further modifications:

1. Corrected braces
2. Removed malloc, work with an array instead
3. After deleting an entry, stay in the list
4. If an item with no more existing path/file is selected, display a splash but stay in the list
5. Properly handle connecting to USB
Comment by Alexander Levin (fml2) - Friday, 31 August 2007, 21:58 GMT
Slightly better behaviour after deleting an entry: if it was the last entry, exit the plugin.
Comment by Alexander Levin (fml2) - Friday, 31 August 2007, 21:59 GMT
Ah, wrong button clicked! Now with the patch.

Loading...