Should be trivial to fix, adding gnome-love keyword.

MMKeysPlugin.cs is under Plugins!

Right, sorry, that's a corner case :-)

Should this be handled via some sort of stop or just as a Play/Pause? I'm a newbie to the Banshee project (new user and new to the sourcecode) but have some C# experience and would like to assist in at least fixing minor bugs until I learn more about the codebase.

Stop can be mapped to set the player engine idle, which is effectively stop.

Alright, I will do that then--I had been hacking around doing it the same way as the Shift-Space "Stop After Finish" action is handled, but it's easy to rewrite that to set it idle.

Created attachment 87099 [details] [review]
The changes I have made to handle the Stop button

This is the output of svn diff

Created attachment 87100 [details] [review]
Additions to SVN to handle Stop button

There's however only one problem here--on my system, the stop button does not actually get handled at all. I can't figure out why...

Created attachment 87101 [details] [review]
Additions to SVN to handle Stop button

There's however only one problem here--on my system, the stop button does not actually get handled at all. I can't figure out why...

Crap, sorry about the double-post. :-\ I didn't think the first one had gone through.

I do not actually want to introduce stop as an action in the UI, because honestly it doesn't make much sense. The hot key handler should just call PlayerEngineCore.Close().

Also, the MMKeys plugin is legacy code. In GNOME 2.18+, hot keys are handled by the org.gnome.SettingsDaemon DBus service. If this serivce does not exist, the old MMKeys plugin is allowed to load.

Thus, you need to patch support in MMKeys, but also in src/Core/Banshee.Base/GnomeMMKeys.cs. Because it's DBus based, there is much less overhead and complexity, so I opted to not make it a plugin at all.

Finally, a few notes on your code specifically.

a) Please do not put "Added by Bojan Rajkovic..." above all your changes. You should instead create a ChangeLog entry noting all of your changes. You of course will be credited in the ChangeLog and then release notes in the next release. Adding comments like these clutter the code. We can use version control to determine exactly which changes were made at a later time.

b) Please conform to the style set forth in the HACKING file at the root of the project. The only offense I see is:

+        private void OnStopAction(object o, EventArgs args)
+        {
+            if (PlayerEngineCore.CurrentState == PlayerEngineState.Idle) {
+                return;
+            }
+            else {
+                PlayerEngineCore.Close();
+                UpdateMetaDisplay();
+            }
+        }
+

It should be:

if(...) {
} else {
}

Noting brace/line placement and spacing. However, logically, else isn't necessary here at all, and to save white space and make code more readable, it should be omitted if only for a matter of style (because if the if evaluates to true, it will always return).

c) There seems to be some cruft in your patch not related to the patch:
src/Plugins/Banshee.Plugins.Recommendation/banshee-plugin-recommendation.schemas.in

This is probably my fault. That change is generated automatically by our build system, and I had not committed it. Just be aware in the future that you don't include files in patches that aren't really a part of the patch as it can be confusing, even though in this case it's more my fault.

d) Finally, when you attach a patch to bugzilla, do not worry about the mime type. Just check the "Patch" box. When this box is checked it will allow the patch to have a status that can be tracked. I have edited your other patches to properly update their status.

Anyway, just some tips since you're new to the project, and we're glad to have you!

it is easier to make STOP do what PAUSE does

So, I hate the "Stop" button. The whole concept is stupid in the context of a computer and software. The only reason it exists is because it's a carry over from old hardware devices like tape players or CD players, where pause may continue to drain the battery or spin a disc or something.

This is why I do not, and have never wanted to introduce "Stop" in the UI. Pause does exactly what you want - stop playing, and then you have the option of resuming from that point if you want. We provide a "restart" option if you want to start a song over, etc.

However, I don't want to map the multimedia key to "Pause." The reason is that if the user has the option of "Stop" and an option of "Pause," their expectations may be two different things for each option. Thus, we should respect that.

The only difference technically between stop and pause in Banshee is that in pause, the pipeline remains open and can resume. Also in the UI, the track appears to still be open (track info shown in the header, playback indication still alive). Stop will close the pipeline and clear the UI.

Either way, the code for this should be just a couple of lines. The point is just that I do not want to expose stop explicitly in the UI at all - just cater to the event if it exists via the keyboard.

ok

Created attachment 87109 [details] [review]
A patch to handle the Stop multimedia key.

Comment on attachment 87109 [details] [review]
A patch to handle the Stop multimedia key.

Aaron summed up exactly what I was going to say about the STOP key being mapped to PAUSE, so here's the next version of the patch.

I removed all the UI element stuff, and just made the keyboard mapping correct.

Created attachment 87110 [details] [review]
Edited changelog.

Also, you mentioned adding a line to the changelog, which I did.
Here's an updated version of the changelog, as a patch.

Created attachment 87112 [details] [review]
Update of the Stop key patch to remove some unneeded code.

Fixed, yet again.

/me is not having a good day.

Created attachment 87113 [details] [review]
Edited changelog to reflect the changes in the patch.

Heh, no need to upload the entire ChangeLog, it's over two years old :) Pasting your entry directly as a comment is best for me, that way I don't have to deal with conflicts in a ChangeLog patch, and can just copy/paste your entry. 

Don't worry about it this time though. Patch looks good, I'll commit it.

Thanks!