You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
270 lines
7.5 KiB
270 lines
7.5 KiB
15 years ago
|
Hacking on Amarok
|
||
|
-----------------
|
||
|
|
||
|
Please respect these guidelines when coding for Amarok, thanks!
|
||
|
|
||
|
* Where this document isn't clear, refer to Amarok code.
|
||
|
|
||
|
|
||
|
This C++ FAQ is a life saver in many situations, so you want to keep it handy:
|
||
|
|
||
|
http://www.parashift.com/c++-faq-lite/
|
||
|
|
||
|
|
||
|
Formatting
|
||
|
----------
|
||
|
* Spaces, not tabs
|
||
|
* Indentation is 4 spaces
|
||
|
* Lines should be limited to 90 characters
|
||
|
* Spaces between brackets and argument functions
|
||
|
* For pointer and reference variable declarations put a space between the type
|
||
|
and the * or & and no space before the variable name.
|
||
|
* For if, else, while and similar statements put the brackets on the next line,
|
||
|
although brackets are not needed for single statements.
|
||
|
* Function and class definitions have their brackets on separate lines
|
||
|
* A function implementation's return type is on its own line.
|
||
|
* amarok.h contains some helpful macros, foreach and foreachType. Use them,
|
||
|
they improve coding style and readability.
|
||
|
|
||
|
Example:
|
||
|
|
||
|
| bool
|
||
|
| MyClass::myMethod( QPtrList<QListViewItem> items, const QString &name )
|
||
|
| {
|
||
|
| if( items.isEmpty() )
|
||
|
| return false;
|
||
|
|
|
||
|
| foreachType( QPtrList<QListViewItem>, items )
|
||
|
| {
|
||
|
| (*it)->setText( 0, name );
|
||
|
| debug() << "Setting item name: " << name << endl;
|
||
|
| }
|
||
|
| }
|
||
|
|
||
|
Header includes should be listed in the following order:
|
||
|
- Amarok includes
|
||
|
- KDE includes
|
||
|
- Qt includes
|
||
|
|
||
|
They should also be sorted alphabetically, for ease of locating them. A small comment
|
||
|
if applicable is also helpful.
|
||
|
|
||
|
Includes in a header file should be kept to the absolute minimum, as to keep compile times
|
||
|
low. This can be achieved by using "forward declarations" instead of includes, like
|
||
|
"class QListView;" Forward declarations work for pointers and const references.
|
||
|
|
||
|
TIP:
|
||
|
Kate/KDevelop users can sort the headers automatically. Select the lines you want to sort,
|
||
|
then Tools -> Filter Selection Through Command -> "sort".
|
||
|
|
||
|
|
||
|
Example:
|
||
|
|
||
|
| #include "amarok.h"
|
||
|
| #include "debug.h"
|
||
|
| #include "playlist.h"
|
||
|
|
|
||
|
| #include "kdialogbase.h" //baseclass
|
||
|
| #include "kpushbutton.h" //see function...
|
||
|
|
|
||
|
| #include "qlistviewitem.h"
|
||
|
| #include "qwidget.h"
|
||
|
|
||
|
|
||
|
Comments
|
||
|
--------
|
||
|
Comment your code. Don't comment what the code does, comment on the purpose of the code. It's
|
||
|
good for others reading your code, and ultimately it's good for you too.
|
||
|
|
||
|
Comments are essential when adding a strange hack, like the following example:
|
||
|
|
||
|
| /** Due to xine-lib, we have to make KProcess close all fds, otherwise we get "device is busy" messages
|
||
|
| * Used by AmarokProcIO and AmarokProcess, exploiting commSetupDoneC(), a virtual method that
|
||
|
| * happens to be called in the forked process
|
||
|
| * See bug #103750 for more information.
|
||
|
| */
|
||
|
| class AmarokProcIO : public KProcIO
|
||
|
| {
|
||
|
| public:
|
||
|
| virtual int commSetupDoneC() {
|
||
|
| const int i = KProcIO::commSetupDoneC();
|
||
|
| Amarok::closeOpenFiles(KProcIO::out[0],KProcIO::in[0],KProcIO::err[0]);
|
||
|
| return i;
|
||
|
| }
|
||
|
| };
|
||
|
|
||
|
|
||
|
For headers, use the Doxygen syntax. See: http://www.stack.nl/~dimitri/doxygen/
|
||
|
|
||
|
Example:
|
||
|
|
||
|
| /**
|
||
|
| * Start playback.
|
||
|
| * @param offset Start playing at @p msec position.
|
||
|
| * @return True for success.
|
||
|
| */
|
||
|
| virtual bool play( uint offset = 0 ) = 0;
|
||
|
|
||
|
|
||
|
Header Formatting
|
||
|
-----------------
|
||
|
General rules apply here. Please keep header function definitions aligned nicely,
|
||
|
if possible. It helps greatly when looking through the code. Sorted methods,
|
||
|
either by name or by their function (ie, group all related methods together) is
|
||
|
great too.
|
||
|
|
||
|
|
||
|
| #ifndef AMAROK_QUEUEMANAGER_H
|
||
|
| #define AMAROK_QUEUEMANAGER_H
|
||
|
|
||
|
| class QueueList : public KListView
|
||
|
| {
|
||
|
| Q_OBJECT
|
||
|
|
|
||
|
| public:
|
||
|
| Queuelist( QWidget *parent, const char *name = 0 );
|
||
|
| ~QueueList() {};
|
||
|
|
|
||
|
| public slots:
|
||
|
| void moveSelectedUp();
|
||
|
| void moveSelectedDown();
|
||
|
| };
|
||
|
|
||
|
#endif /* AMAROK_QUEUEMANAGER_H */
|
||
|
|
||
|
|
||
|
0 vs NULL
|
||
|
---------
|
||
|
The use of 0 to express a null pointer is preferred over the use of NULL.
|
||
|
0 is not a magic value, it's the defined value of the null pointer in C++.
|
||
|
NULL, on the other hand, is a preprocessor directive (#define) and not only is
|
||
|
it more typing than '0' but preprocessor directives are less elegant.
|
||
|
|
||
|
| SomeClass *instance = 0;
|
||
|
|
||
|
|
||
|
Const Correctness
|
||
|
-----------------
|
||
|
Try to keep your code const correct. Declare methods const if they don't mutate the object,
|
||
|
and use const variables. It improves safety, and also makes it easier to understand the code.
|
||
|
|
||
|
See: http://www.parashift.com/c++-faq-lite/const-correctness.html
|
||
|
|
||
|
|
||
|
Example:
|
||
|
|
||
|
| bool
|
||
|
| MyClass::isValidFile( const QString& path ) const
|
||
|
| {
|
||
|
| const bool valid = QFile::exist( path );
|
||
|
|
|
||
|
| return valid;
|
||
|
| }
|
||
|
|
||
|
|
||
|
Debugging
|
||
|
---------
|
||
|
debug.h contains some handy functions for our debug console output.
|
||
|
Please use them instead of kdDebug().
|
||
|
|
||
|
Usage:
|
||
|
|
||
|
| #include "debug.h"
|
||
|
|
|
||
|
| debug() << "Something is happening" << endl;
|
||
|
| warning() << "Something bad may happen" << endl;
|
||
|
| error() << "Something bad did happen!" << endl;
|
||
|
|
||
|
Additionally, there are some macros for debugging functions:
|
||
|
|
||
|
DEBUG_BLOCK
|
||
|
DEBUG_FUNC_INFO
|
||
|
DEBUG_LINE_INFO
|
||
|
DEBUG_INDENT
|
||
|
DEBUG_UNINDENT
|
||
|
|
||
|
AMAROK_NOTIMPLEMENTED
|
||
|
AMAROK_DEPRECATED
|
||
|
|
||
|
threadweaver.h has two additional macros:
|
||
|
DEBUG_THREAD_FUNC_INFO outputs the memory address of the current QThread or 'none'
|
||
|
if its the original GUI thread.
|
||
|
SHOULD_BE_GUI outputs a warning message if it occurs in a thread that isn't in
|
||
|
the original "GUI Thread", otherwise it is silent. Useful for documenting
|
||
|
functions and to prevent problems in the future.
|
||
|
|
||
|
|
||
|
Usage of Amarok::config()
|
||
|
-------------------------
|
||
|
We provide this method for convenience, but it is important to use it properly. By
|
||
|
inspection, we can see that we may produce very obscure bugs in the wrong case:
|
||
|
|
||
|
| KConfig
|
||
|
| *config( const QString &group )
|
||
|
| {
|
||
|
| //Slightly more useful config() that allows setting the group simultaneously
|
||
|
| kapp->config()->setGroup( group );
|
||
|
| return kapp->config();
|
||
|
| }
|
||
|
|
||
|
Take the following example:
|
||
|
|
||
|
| void
|
||
|
| f1()
|
||
|
| {
|
||
|
| KConfig *config = Amarok::config( "Group 2" );
|
||
|
| config->writeEntry( "Group 2 Variable", true );
|
||
|
| }
|
||
|
|
|
||
|
| void
|
||
|
| doStuff()
|
||
|
| {
|
||
|
| KConfig *config = Amarok::config( "Group 1" );
|
||
|
| f1();
|
||
|
| config->writeEntry( "Group 1 Variable", true );
|
||
|
| }
|
||
|
|
||
|
We would expect the following results:
|
||
|
|
||
|
| [Group 1]
|
||
|
| Group 1 Variable = true
|
||
|
|
|
||
|
| [Group 2]
|
||
|
| Group 2 Variable = true
|
||
|
|
||
|
However because the config group is changed before writing the entry:
|
||
|
| [Group 1]
|
||
|
|
|
||
|
| [Group 2]
|
||
|
| Group 1 Variable = true
|
||
|
| Group 2 Variable = true
|
||
|
|
||
|
Which is clearly incorrect. And hard to see when your wondering why f1() is not
|
||
|
working. So do not store a value of Amarok::config, make it a habit to just
|
||
|
always call writeEntry or readEntry directly.
|
||
|
|
||
|
Correct:
|
||
|
| amarok::config( "Group 1" )->writeEntry( "Group 1 Variable", true );
|
||
|
|
||
|
|
||
|
Errors & Asserts
|
||
|
----------------
|
||
|
*Never use assert() or fatal(). There must be a better option than crashing a user's
|
||
|
application (its not uncommon for end-users to have debugging enabled).
|
||
|
|
||
|
*KMessageBox is fine to use to prompt the user, but do not use it to display errors
|
||
|
or informational messages. Instead, KDE::StatusBar has a few handy methods. Refer to
|
||
|
amarok/src/statusbar/statusBarBase.h
|
||
|
|
||
|
|
||
|
Copyright
|
||
|
---------
|
||
|
To comply with the GPL, add your name, email address & the year to the top of any file
|
||
|
that you edit. If you bring in code or files from elsewhere, make sure its
|
||
|
GPL-compatible and to put the authors name, email & copyright year to the top of
|
||
|
those files.
|
||
|
|
||
|
|
||
|
Thanks, now have fun!
|
||
|
-- the Amarok developers
|