On Monday 31 of October 2011, Timothy Pearson wrote:
> > And I don't remember any reply to my questions, and I don't see anything
> > changing since then, so they still stand: What are the reasons this is
> > done as a huge copy&paste s/KDE/TDE/ patch?
>
> Primarily so that we can start hacking on the TDE module without breaking
> the KDE3 module, and also if you look at the patch the Qt classnames have
> all been altered. The TDE developers are nearing the end of a yearlong
> project to convert TDE to use those class names, and as soon as the final
> transition is done the KDE3 module will be completely unbuildable, let
> alone usable, on TDE.
If it's really only class name changes, then it's nothing that a couple of
typedef's or #define's wouldn't solve.
> I have been discussing this with Michael Meeks, and basically if
> LibreOffice would rather we hack up the KDE3 module to work with TDE that
> is fine, but we will not be able to guarantee that our changes do not
> seriously break the module when used under KDE3. I have serious concerns
> about the maintainability of such code, including readability,
I picked a file at random, vcl/unx/kde/salnativewidgets-kde.cxx, compared it
with the TDE file from the patch, and if I ignore type name changes, the
differences boil down to the attached a.patch, which is 2 changes of less
than 20 lines in total, for a file that is 70k big. Second random file
picked, just in case, fpicker/source/unx/kde/kdefilepicker.cxx , is pretty
much identical (again minus the type name changes).
In other words, to me it looks like it would have been much less work to
create the TDE version by something like:
foo-tde.cxx:
#define BUILDING_TDE
#define QApplication QTApplication
...
#include "foo-kde.cxx"
foo-kde.cxx:
...
#ifdef BUILDING_TDE // or possibly even better "#if TDE_IS_VERSION( 3,7,0 )"
...
#endif
...
> speed of patch acceptance (due to offsite KDE3 tests most likely being
> mandatory), and how prone the code will be to accidental breakage.
If you start taking care of the KDE3/TDE code, you'll more or less end up
being the maintainer of it. People occassionally do a build fix or similar, I
rarely do a bugfix when I find the time and/or somebody whines a lot, but
that's about it I expect.
I would assume that TDE still maintains some kind of backwards
compatibility[*], so you should generally know whether your changes are safe
or whether it's something new that needs an #ifdef (in which case the
TDE_IS_VERSION check is needed anyway). If something slips through, it can
get fixed when somebody notices.
So I don't know the extent of the changes you plan in the code, but from what
I can see there does not seem to be a good reason to have a special copy of
the sources for TDE.
[*] I still don't see it said anywhere if TDE maintains any API backwards
compatibility with KDE3 or not. The configure checks in the patch search for
KDE3/Qt3 names, so I would assume the answer is yes. If not, then at least
that part of the patch is definitely wrong.
--
Lubos Lunak
l.lu...@suse.cz
--- /home/llunak/build/src/libo/vcl/unx/kde/salnativewidgets-kde.cxx 2011-10-31 13:32:50.598155002 +0100
+++ a.cxx 2011-11-01 17:57:00.640561003 +0100
@@ -392,11 +395,7 @@ WidgetPainter::WidgetPainter( void )
m_pToolBarVert( NULL ),
m_pToolButton( NULL ),
m_pMenuBar( NULL ),
- m_nMenuBarEnabledItem( 0 ),
- m_nMenuBarDisabledItem( 0 ),
m_pPopupMenu( NULL ),
- m_nPopupMenuEnabledItem( 0 ),
- m_nPopupMenuDisabledItem( 0 ),
m_pProgressBar( NULL )
{
}
@@ -1923,7 +1922,17 @@ void KDESalFrame::UpdateSettings( AllSet
aStyleSettings.SetFaceColor( aBack );
aStyleSettings.SetInactiveTabColor( aBack );
aStyleSettings.SetDialogColor( aBack );
- aStyleSettings.SetCheckedColorSpecialCase( );
+ if( aBack == COL_LIGHTGRAY )
+ aStyleSettings.SetCheckedColor( Color( 0xCC, 0xCC, 0xCC ) );
+ else
+ {
+ Color aColor2 = aStyleSettings.GetLightColor();
+ aStyleSettings.
+ SetCheckedColor( Color( (BYTE)(((USHORT)aBack.GetRed()+(USHORT)aColor2.GetRed())/2),
+ (BYTE)(((USHORT)aBack.GetGreen()+(USHORT)aColor2.GetGreen())/2),
+ (BYTE)(((USHORT)aBack.GetBlue()+(USHORT)aColor2.GetBlue())/2)
+ ) );
+ }
// Selection
aStyleSettings.SetHighlightColor( toColor( qColorGroup.highlight() ) );
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
