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.

* CamelCase.{cpp,h} style file names.

* Qt 4 includes a foreach keyword which makes it very easy to iterate over all

  elements of a container.

Example:

 | bool

 | MyClass::myMethod( QStringList list, const QString &name )

 | {

 |     if( list.isEmpty() )

 |         return false;

 |

 |     /*

 |       Define the temporary variable like this to restrict its scope

 |       when you do not need it outside the loop. Let the compiler

 |       optimise it.

 |      */

 |     foreach( const QString &string, list )

 |         debug() << "Current string is " << string << endl;

 | }

Using "astyle" for auto formatting

----------------------------------

The program astyle can be used to automatically format source code, which can

be useful for badly formatted 3rd party patches.

Use it like this to get (approximately) Amarok formatting style:

    "astyle -s4 -b -p -U -D -o source.cpp"

Class, Function & Variable Naming

---------------------------------

*Use CamelCase for everything.

*Local variables should start out with a lowercase letter.

*Class names are captialized

*Prefix class member variables with m_, ex. m_trackList.

*Prefix static member variables with s_, ex s_instance

*Functions are named in the Qt style. It's like Java's, without the "get" 

 prefix.

  *A getter is variable()

  *If it's a getter for a boolean, prefix with 'is', so isCondition()

  *A setter is setVariable( arg ).

Includes

--------

Header includes should be listed in the following order:

    - Own Header

    - 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".

In vim the same can be achieved by marking the block, and then doing ":sort".

Example:

 | #include "MySuperWidget.h"

 |

 | #include "Amarok.h"

 | #include "Debug.h"

 | #include "Playlist.h"

 |

 | #include <KDialogBase>    //baseclass

 | #include <KPushButton>    //see function...

 |

 | #include <QGraphicsView>

 | #include <QWidget>

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 K3Process 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 K3ProcIO 

 | {

 |     public:

 |     virtual int commSetupDoneC() {

 |         const int i = K3ProcIO::commSetupDoneC();

 |         Amarok::closeOpenFiles(K3ProcIO::out[0],K3ProcIO::in[0],K3ProcIO::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. Access levels should be sorted in this order: public, protected, private.

 | #ifndef AMAROK_QUEUEMANAGER_H

 | #define AMAROK_QUEUEMANAGER_H

 | class QueueList : public K3ListView

 | {

 |         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 kDebug().

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.

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

Commenting Out Code

-------------------

Don't keep commented out code. It just causes confusion and makes the source

harder to read. Remember, the last revision before your change is always

availabe in SVN. Hence no need for leaving cruft in the source.

Wrong:

 | myWidget->show();

 | //myWidget->rise(); //what is this good for?

Correct:

 | myWidget->show();

Tips & Tricks

-------------

A useful service is http://lxr.kde.org. Lxr is a cross reference of the entire

KDE SVN repository. You can for instance use it to search for example code

from other applications for a given KDElibs method.

Markey's .vimrc

---------------

let ruby_no_expensive = 1

syntax on

set shiftwidth=4

set tabstop=4

set expandtab

set hlsearch

set ruler

set smartindent

set nowrap

set ignorecase

set smartcase

set title

set showtabline=2  "Makes the status bar always show, also for just one tab

autocmd FileType ruby set shiftwidth=2 tabstop=2

Git and SVN aware prompt

----------------

The following prompt shows the current git branch if sitting in a git repository.

Random crap courtesy of shell colours.

export PS1='\[\033[01;32m\]\u@\h\[\033[00m\]:\[\033[01;34m\]\w\[\033[00m\]\[\033[01;33m\]`git branch 2>/dev/null|cut -f2 -d\* -s`\[\033[00m\]\$ '

This is an even more colorful configuration for .bashrc that displays the

current git branch if sitting in a git repository and the current SVN revision

if sitting in an SVN checkout.

svn_prompt() {

	SVNBRANCH=`svn info 2>/dev/null | grep Revision: | cut -f 2 -d " "`

	if [ -n "$SVNBRANCH" ]; then

		BRANCH=" (r$SVNBRANCH)";

	else

		BRANCH="";

	fi

    echo "$BRANCH"

}

export PS1='\[\e[0;32m\]\u@\h\[\e[m\] \[\e[1;34m\]\w\[\e[m\]\[\033[01;33m\]`__git_ps1`\[\e[m\]\[\e[01;35m\]`svn_prompt`\[\e[m\]\[\033[00m\]  \[\e[1;32m\]\$ \[\e[m\]\[\e[1;37m\] '

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.

Please note that it is not sufficient to write a pointer to the license (like a URL).

The complete license header needs to be written everytime.

Thanks, now have fun!

  -- the Amarok developers