<div dir="ltr"><span style="font-family:arial,sans-serif;font-size:13px">It wasn't supposed to be private, that was accidental.</span><div><font face="arial, sans-serif">Resending it to the amarok-devel mailing list.<br>
</font><div><span style="font-family:arial,sans-serif;font-size:13px"><br>Thanks for the thorough review :)</span><div class="im" style="font-family:arial,sans-serif;font-size:13px"><br><br>On Tue, Apr 30, 2013 at 3:56 AM, Matěj Laitl <span dir="ltr"><<a href="mailto:matej@laitl.cz" target="_blank">matej@laitl.cz</a>></span> wrote:<br>
</div><div class="gmail_extra" style="font-family:arial,sans-serif;font-size:13px"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>On 30. 4. 2013 Anmol Ahuja wrote:<br>> And for classes like CollectionLocation, should I create wrappers like<br>> CollectionLocationTypeExporter or simple modify the existing class,<br>> changing script evokable functions to public slots/ Q_INVOKABLE? I think<br>
> the former would help keep the scripting logic in one place.<br><br></div>Yeah, the wrappers like the MetaTypeExporter (which should be renamed to<br>TrackExporter by the way) should be preferred.<br></blockquote><div>
</div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> Short description : My proposal aims at revamping the Amarok scripting<br>> interface, and adding an intuitive Script Creator graphical application<br>> allowing easy script creation by non-coders and coders alike.<br>
<br></div>Please update this to reflect our Saturday IRC discussion - i.e. removal of the<br>Script Creator from the project.<br></blockquote><div> </div></div><div>Okay.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> This involves<br>> getting rid of dangerous and obsolete methods, and adding accessors for all<br>> the new features added to Amarok over the years. Every major type will have<br>> a QtScript object representation, allowing unparalleled scripting<br>
> flexibility. I also plan on writing unit tests to allow automated testing<br>> of the Amarok scripting API. The third part involves documentation of the<br>> entire scripting API in-source and automating the generation of Amarok<br>
> Scripting API documentation from the in-source documentation. All of this<br>> will lead to a cleaner and more feature rich well-documented scripting API<br>> hopefully generating new interest in Amarok script.<br>
<br></div>Yeah, this all sounds nice. I'd say "scripting" in the last sentence. And you<br>may want to work on documentation generation right from the start so that you<br>can check generated documentation for classes you just create.<br>
<br>There may be some problems with the generation (because you are documenting in<br>C++, while the target code is Javascript - but I believe that with some help<br>from doxygen people, docs (and perhaps the guy who wrote qstestlib) and some<br>
work-arounds this will be possible. Feel free to rename class names to match<br>their names in the scripting API, i.e. I don't understand why<br>AmarokScript::AmarokEngineScript cannot be called just AmarokScript::Engine)<br>
</blockquote><div> </div></div><div>Okay, starting with the documentation generation first. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> Other features I plan<br>> to work upon, if time permits, are allowing users to terminate long running<br>> or stuck scripts instead of forcing a restart, adding GUI handling to the<br>> script creation utility and porting popular Amarok 1.4 scripts, if<br>
> possible, to the newer QtScript based interface.<br><br></div>Right, although I'd remove the "porting popular A 1.4 scripts" - there are<br>more useful things you'll be able to do, like promoting Amarok scripting,<br>
</blockquote><div> </div></div><div>I'm not certain about how I would promote Amarok scripting. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
getting feedback on scripting interface from existing script authors etc.<br></blockquote><div> </div></div><div>I am interested in knowing what the Amarok scripting community wants. Can I find Amarok scripters on #amarok? Can I set up an Amarok Script feature request poll on the Amarok website? </div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>> Freenode IRC Nick : darthcodus<br>
<br>"DarthCodus"? (Linux is case-sensitive, hehe ;) )<br></blockquote><div> </div></div><div>Silly me :p</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> IM Service and Username : Gtalk, darthcodus<br><br></div><a href="mailto:darthcodus@gmail.com" target="_blank">darthcodus@gmail.com</a>?<br><div><br>> Motivation for Proposal:<br>><br>> Amarok has a thriving scripting community, but the existing scripting API<br>
> is:<br></div>> <------- shortened ----------><br><br>Yes, right, very nice research! Kudos for including bug links.<br><div><br>> Project Goals:<br>><br>> My proposal involves four main parts :<br>
</div>
> - Part I : (...)<br><div>> Remove AmarokCollectionScript and re-implement as QScriptObject<br><br></div>Hmm, that may end up being tricky. I propose you have this as a soft goal at<br>the very end.</blockquote>
<div> </div></div><div>I'm not exactly doing that anymore. I'm following the outline in the implementation details below, so it's manageable.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>> Remove direct SQL access<br><br>Please note that all the current scripts must still work, so you cannot simply<br>remove any method/class (yet). What you should do is to deprecate them in a<br>very visible way, my idea would be to introduce a helper function that would<br>
emit following message Amarok::Compontents::logger->longMessage():<br><br>The script <script name> (homepage: <clickable link>) uses deprecated method<br><method name>. Please try to update to newest version of the script in Amarok<br>
Config -> Scripts -> Manage Scripts. If you are already using the newest<br>version please contact the author at <author addres>. The mentioned method may<br>be removed in next Amarok release. See Amarok debugging log for complete list<br>
of used deprecated methods.<br><br></blockquote><div> </div></div><div>I actually did intend deprecation, though it was poorly put in the proposal :)</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
^^^ such message would appear only once per script, until Amarok is restarted.<br><br>> - CollectionLocation<br>> - Transcoding<br><br>Yeah, transcoding may end up just as some parameter of a method in<br>
CollectionLocation, although it may be useful<br></blockquote><div> </div></div><div>I can see uses for a transcoding controls in Amarok Script. For instance, you can have a script that uploads any song you love on Last.FM to be uploaded to your Google Music account, transcoded to MP3 if, say, it is FLAC or Apple Lossless, not otherwise. This sort of stuff is not configurable from the main interface, not yet at least.</div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>> QueryMaker<br>
<br>Yeah, this is much needed, although the Javascript API may be simplified<br>compared to the C++ API. I thought about using the textual query parser from<br>the Collection Browser so that the script authors may use something they are<br>
familiar with and can test easily.<br></blockquote><div> </div></div><div>Okay. I'll include a textual query parser in the next version of the proposal. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> Implement Equalizer support in the AmarokEngineScript<br><br></div>See related <a href="https://git.reviewboard.kde.org/r/110036/" target="_blank">https://git.reviewboard.kde.org/r/110036/</a> (on the other hand this<br>
may require some kind of EqualizerControlelr on the C++ side - maybe make this<br>a soft goal)<br></blockquote><div> </div></div><div>It would actually require modifications on the C++ side, but that's manageable enough.</div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>> - Synchronization Job<br>
<br>Do you mean statistics synchronization? Perhaps even more useful would be to<br>expose "Track matching job" is some way.<br><div><br></div></blockquote><div> </div></div><div>Okay, I'll include Track matching job in the next revision. The SynchronizationJob is for playlist synchronization and stuff, though admittedly I'm not too familiar with that part of Amarok.</div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>> Others<br>
> Add suspend inhibition to AmarokScript<br>
<br></div>Hmmm, I don't think we should support this - the inhibition is pretty much<br>defined by the playback.<br><div><br></div></blockquote><div> </div></div><div>This is to allow scripts to inhibit suspend, for example, when they are downloading something. Or is that actually bad? Maybe we should have a popup show up in case a script is inhibiting suspend so as not to perplex users?</div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>> Add other requested features/ wishes and fix bugs<br>
<br></div>Nice research, although some users may request functionality that we simply<br>don't want to expose.<br></blockquote><div> </div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>> Bug 213376 <<a href="https://bugs.kde.org/show_bug.cgi?id=213376" target="_blank">https://bugs.kde.org/show_bug.cgi?id=213376</a>> - Implement<br>> setter for Amarok.Engine.currentTrack().imageURL<br>
<br>Isn't this already done by MetaTypeExporter::setImageUrl()?<br></blockquote><div> </div></div><div>It's not yet implemented. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>> - Bug 183497 <<a href="https://bugs.kde.org/show_bug.cgi?id=176663" target="_blank">https://bugs.kde.org/show_bug.cgi?id=176663</a>> -<br><div>> Scripting API to add a context menu to everything representing a song<br>
> or file<br></div>> - Bug 187957 <<a href="https://bugs.kde.org/show_bug.cgi?id=187957" target="_blank">https://bugs.kde.org/show_bug.cgi?id=187957</a>> - Create<br><div>> submenus from the Scripting interface<br>
</div>> - Bug 176663 <<a href="https://bugs.kde.org/show_bug.cgi?id=176663" target="_blank">https://bugs.kde.org/show_bug.cgi?id=176663</a>> -<br><div>> Installing a new script requires a restart of amarok<br>
</div>> - Bug 23271 <<a href="https://bugs.kde.org/show_bug.cgi?id=232711" target="_blank">https://bugs.kde.org/show_bug.cgi?id=232711</a>>: - Provide<br><div>> scripting interface to customize collection display (filter, sort,<br>
> group, displayed tags, etc.)<br></div>> - Bug 205509 <<a href="https://bugs.kde.org/show_bug.cgi?id=205509" target="_blank">https://bugs.kde.org/show_bug.cgi?id=205509</a>> : Add<br><div>> dbus functions to update podcasts and download podcast tracks<br>
<br></div>These are for the D-Bus API instead, but yes, perhaps the reported would be<br>happy with the Javascript API.<br></blockquote><div> </div></div><div>I was actually planning on adding those to the D-BUS too. Should I keep it out of my proposal? </div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><br>> Add new scripts demonstrating the new scripting API ( and adding cool<br>
> new features )<br></div>> - A generic full-fledged synchronization framework like iTunes written<br><div>> in Amarok Script for media devices<br></div>> - Link to Google Music Search for currently playing artist (also Bug<br>
> 136502 <<a href="https://bugs.kde.org/show_bug.cgi?id=136502" target="_blank">https://bugs.kde.org/show_bug.cgi?id=136502</a>>)<br><div>> Automatic adjust equalizer according to genre (also Bug<br>
</div>> 129374<<a href="https://bugs.kde.org/show_bug.cgi?id=129374" target="_blank">https://bugs.kde.org/show_bug.cgi?id=129374</a>><br>> )<br><br>Please make this a soft-goal with rather low priority.<br>
</blockquote><div> </div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> Part II : Design a graphical Script Creator utility<br>><br>> Create a user-friendly script creation GUI allowing everybody, coders and<br>> non-coders alike to write scripts for Amarok. The utility will be generic<br>
> enough to allow easy porting to other KDE apps.<br><br></div>Please remove this from the project. You might want to make existing Amarok<br>Scripting Console more useful and convenient, but no brand new GUI app please.<br>
<div><br>> Part III : Automated Testing Scripts<br>><br>> Write scripts for unit-testing the scripting interface. Look into existing<br>> solutions like QSTestLib (<br>> <a href="http://blog.qt.digia.com/blog/2007/11/05/unit-testing-with-qt-script/" target="_blank">http://blog.qt.digia.com/blog/2007/11/05/unit-testing-with-qt-script/</a> ) to<br>
> see if they’re viable for<br><br></div>unfinished sentence?<br></blockquote><div> </div></div><div>I'm sorry, I should've had a closer look at it before mailing it.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>> Part IV: Automating Documentation<br>> - Document any undocumented scripting API code<br><br>I don't think we have a documented scripting API class/method. </blockquote><div> </div></div><div>A few of them do have in source documentation. </div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>> - Use Doxygen for generating the wiki Scripting API Documentation content<br>
<div>> from in source documentation. [Or should I write my own scripts for<br>> that?]<br><br></div>No, Doxygen was the intent, altough you may have to fiddle with it a bit so<br>that it produces desired results.<br>
</blockquote><div> </div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br><br>> Implementation Details //incomplete, is all this really needed?<br><br></div>Not really, this is too detailed and you already provided some details in the<br>Project Goals section. Maybe mention just some interesting changes/removals,<br>
certainly not lists of all methods.<br></blockquote><div> </div></div><div>Okay. I'll also add a class diagram or two concisely representing this stuff.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>Instead of this, it may be really helpful if you provided a brief outline how<br>general QtScript - Javascript - C++ interactions works, what QtScript classes<br>are you likely to be using, what is the role of QObject, what are the memory<br>
management options etc. - so that we see that you understand the principles<br>and can have confidence in you.<br></blockquote><div> </div></div><div>Okay. I should've realized it after reading you review of Konrad's proposal. </div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><br>> *Enums will be handled as strings in Amarok Scipt, converted to and fro<br>
</div>> using static enum to string maps*<br><br>^^ Is this the best practice when creating QtScript bindings? How does<br>qtscriptgenerator (you should check this project, it is used to generate<br>bindings for actual Qt classes - I don't mean directly use it, but at least<br>
see how it converts C++ things to Javascript)<br></blockquote><div> </div></div><div>I didn't know I could directly grant Enums access with the QMetaObject system! Thanks.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>> 1. AmarokScript<br>><br>> public signals:<br>> configured (I’m actually uncertain when it’s supposed to be emitted, when<br>> the Amarok configuration changes? )<br><br></div>Yes, but it isn't connected to anything recently. Perhaps remove it for now, a<br>
signal can be always added should the need arise.<br></blockquote><div> </div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>> 2. AmarokEngineScript<br><div>> stopped(); paused(); //needed? can be inferred from playbackStateChanged();<br><br></div>I'd preserve these for convenience.<br></blockquote><div> </div></div><div>They don't already exist, I was wondering whether I should add them, since they can be inferred from engineState(), which already exists (not playbackStateChanged() signal as mentioned above, that's erroneous). I'll add them as properties. (The implementation details only list properties and signals/ slots that don't already exist)</div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>> 3. AmarokEqualizerScript<br>
<br>Needs some more thinking, no need to do it in the proposal.</blockquote><div> </div></div><div>Is this unnecessary? I can't think of much use for the scriptable equalizer either, except for things like mood scripts and switching equalizer preset on inserting headphones (headphone detection isn't in the Amarok code yet, either, and I'm not even sure if it's possible with Solid). But there was a <a href="https://bugs.kde.org/show_bug.cgi?id=279701" target="_blank">feature request</a> for this, and it seems pretty reasonable to expose it.</div>
<div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> 5. AmarokScriptableServiceScript<br>><br>> StreamItem and ScriptableServiceScript in same header file, not in<br>> AmarokScript namespace where they should be<br><br></div>Right. (better wording would be to say how you'll fix it (move to bla bla bla)<br>
rather than what the current problem is)<br></blockquote><div> </div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> 7. Handling collections, copying between collections ,transcoding and<br>> querying<br>><br>> Implement in a more object oriented way.<br><br></div>Right.<br><div><br>> [ global CollectionManager object - Amarok.CollectionManager ]<br>
<br></div>Right.<br><div><br>> public:<br>> enum Type<br>> {<br>> UmsCollection,<br>> IpodCollection,<br></div>> (...)<br><br>No no no, this is artificial and no such enum actually exists in C++ code.<br>
Instead, there should be just a method to get a list of collections (please<br>hide the distinction between viewable and whatever else, just return the<br>"active" collections).<br></blockquote><div> </div></div>
<div>Okay. I'll just use a type field so script authors can identify the type of collection.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br><br>> b. class CollectionTypeExporter<br><br>Right.<br><br>> c. TranscodingConfigurationTypeExporter<br><br>Perhaps this won't be needed? I don't know, maybe postpone this until we get<br>an idea whether yes or no.</blockquote>
<div> </div></div><div>Justified above. </div><div><div class="adm"><div id="q_13e5b49284bfb083_61" class="h4"><div class="" style></div></div></div></div><div>Okay. </div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>> <----- All components of the scripting interface ready at this point-----><br>><br>> August 31 - September 9 : Write unit tests for the Amarok scripting<br>> interface<br>><br>> September 10 - September 16 : Write missing documentation, set up automatic<br>
> documentation generation<br><br></div>Hmm, I'd like to see setting up automatic doc generation as a very first task,<br>not last. I'd also like to see that writing documentation for existing methods<br>should be a part of the initial cleanup and I think you should devote more<br>
time for it (perhaps 2 weeks for the cleanup + documentation).<br><br>I also recommend writing the unit-tests right when some API class/method is<br>finished, not in batch at the end. Writing documentation right with the<br>
implementation should be automatic. Because of that, I think it will take more<br>time to get a new class API finished that you expect in above timeline.<br></blockquote><div> </div></div><div>Okay. </div><div><div class="adm">
<div id="q_13e5b49284bfb083_65" class="h4"><div class="" style></div></div></div></div><div>Can I move Script Creator to the bottom of the priority list? I won't waste too many lines of the proposal on it either.</div>
<div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">provide the requested overview of how QtScript generally works, dramatically<br>
shorten the Implementation Details section, resolve other minor remarks and<br>this will be a really good one.<br><br>Regards,<br> Matěj<br></blockquote><div> </div></div></div><div class="gmail_extra">I will be submitting a new version of the proposal soon.</div>
<div class="gmail_extra">Thanks again :)<br></div></div><div class="gmail_extra" style="font-family:arial,sans-serif;font-size:13px"><br></div><div class="gmail_extra" style="font-family:arial,sans-serif;font-size:13px">
--</div>
<div class="gmail_extra" style="font-family:arial,sans-serif;font-size:13px">Anmol</div></div></div></div>