patch for src/playlist/view...

[Nikolaj Hald Nielsen]

I have committed your patch. I might tweak some things, but overall it
looks sane! :-)

Thanks!

- Nikolaj

On Sat, Jan 10, 2009 at 7:45 PM, Thomas Lübking <thomas.luebking at web.de> wrote:
> Am Saturday 10 January 2009 schrieben Sie:
>> Hi Thomas,
> Hi Nikolaj (you get the mail twice to avoid list approval delay... ;-)
>
>> Thanks for the patch. I just added all this new playlist code from a
>> personal git branch the other day, so I am well aware that it needs a
>> lot of polish still! :-)
> actually i've found some more issues - partially from my patch :-(
>
> ... see new patch =D
>
> 1) the model values were escaped from the playlist model
> ----
> ...leading to &s; etc. in the playlist
>
> maybe it would be better to pass the plain version from the model and escape
> it on demand? (i.e. if the view uses richtext painting - afaics currently
> none
> does?)
> otherwise we'd need to QString::replace() every playlist entry after just
> uselessy escaping it :-(
>
> 2) (fallen out) there was an issue with empty etries (e.g. i tried to
> embrace
> the album, leading to an opening brace but no closing one and no dummy
> content)
>
> leading to
>
> 3) there's no dummy ("unknown") content if the metafield is empty, what
> (though a good idea in general) can lead to dull output like
> " by on "<album> ... ;-P
>
> i therefore extended the elements by two attributes "prefix" and "suffix" to
> be able to bind synthetic text to specific metainfo and removed the "value"
> string handling (which was incoherent anyway)
>
> 4) i tried a better handling for right aligned elements (kinda bidi
> rendering)
> when using autosizes (before, everything is ditched to the right border)
>
> 5) the casual padding extension was afaics useless (you can add the padding
> anyway - it's just not gonna used for the last element at all...)
>
> more TODO:
> ==========
> - if you accept the below patch, the "Title (with Track number) attribute
> can
> probably be removed (e.g. i currently use a dot)
>
> - the tracknumber should be "offsettable" (i.e. check max numbers in album
> and
> prefix a "0" - for a simple life, we could just use %2d anyway, assuming
> that
> most albums will have 10 - 99 tracks, esp. as afair 99 is a cdda limit...)
>
> - we should hide out for a way to inject the album length for the "length"
> attribute in headers
>
> - the last used layout should be stored and reused on quit/restart
>
> - the xml file should be reparsed on layout changes (currently i had to
> restart amarok everytime i changed the least bit in the config file)
>
>> If you are interested, you are more than welcome to help improving it
>> and making sure that Amarok 2.1.0 gets the coolest playlist of any
>> player out there. The major task that needs doing is coming up with a
>> simple and intuitive interface for letting users configure elements
> kwrite ;-P
>
> ok, for the weak and unskilled: i guess you've seen the new kmail listview
> (actually i thought amarok and kmail were gonna rely on the same lib)
> it already has a nice configurator. maybe you can just dump that code and
> use
> amarok specific items? (at least the result is very similar but i don't know
> if it manages ann xml structure as well - KDE could really need such class
> however)
>
>> Also, do you have an example layout using some of the new features (
>> auto sizing and custom strings ) that your patch added? If so, could
>> you please send a patch with that?
> attached (in addition to the patch)
>
>> usually catch me on IRC channel #amarok on irc.freenode.net . My nick
>> is nhnFreespirit. I will most likely be away most of today though.
> me == unreasonable internet connection (for teh rest of this month -
> hopefully) == nix IRC ;-)
> i'll contact you there /iff/ i enter this century before we did the
> playlist...
>
> Thomas
>
>
>
>