From konsole-devel Tue Feb 10 18:33:45 2009 From: Andrius da Costa Ribas Date: Tue, 10 Feb 2009 18:33:45 +0000 To: konsole-devel Subject: Re: [Konsole-devel] [PATCH] Windows port Message-Id: X-MARC-Message: https://marc.info/?l=konsole-devel&m=123429088509347 Is it possible to include a preview binary in the installer? 2009/2/1 Robert Knight : > Hi Patrick, > > Seems sensible to me but it is probably best to discuss issues around > dividing the code into libraries on kde-devel@kde.org > where there are developers who understand this better than I do. > > Regards, > Robert. > > > 2009/2/1 Patrick Spendrin : >> Robert Knight schrieb: >>> >>> Hi, >>> >>> Thank-you for the patch. I prefer platform specific #if-defs to be >>> kept to a minimum. If you are writing code which would compile on all >>> platforms but should only be executed on some of them, eg: >>> >>> +#ifndef Q_WS_WIN >>> setPtyChannels(KPtyProcess::AllChannels); >>> +#else >>> + setOutputChannelMode(SeparateChannels); >>> +#endif >> >> Especially this code doesn't compile here ;-) (due to missing KPtyProcess) >> and most of the code used for real terminal interaction doesn't compile >> here, so it surely won't work without ifdefs (but you're right, we can >> minimize them). >> A patch for that needs me some more time though. >>> >>> Then it would be better to put the platform specific code in a >>> sub-class and call it via a virtual function. This is easier to read >>> and compile-time breakages in code for other platforms are detected >>> sooner - see the ProcessInfo class implementations for Linux and >>> Solaris. Things are slightly more complex here since you want to use >>> KProcess as the base class on Windows and KPtyProcess on Unix. That >>> could be fixed by making Konsole::Pty not inherit directly from >>> KProcess but have a KProcess as a field internally and provide a >>> virtual process() accessor. >>> >>> +#ifndef Q_WS_WIN >>> class KDE_EXPORT Profile : public QSharedData >>> +#else >>> +class Profile : public QSharedData >>> +#endif >>> >>> Why is KDE_EXPORT being removed here on Windows? If it is required, >>> the KDE_EXPORT macro should be replaced with something else (eg. >>> KONSOLE_TEST_EXPORT) which is #defined appropriately depending on the >>> platform. >> >> I am currently trying to find a way through this. >> The small problem is that there is a difference between a kdeinit executable >> under Windows and under Linux. the first is a normal application, whereas >> the second is more of the library kind. This also makes linking to the >> kdeinit executable possible under Linux (and that way also exporting from >> it) whereas this doesn't work under windows. >> >> Edit: I decided to make up a libkonsoleprivate instead which is linked by >> the tests, the kdeinit executable and the kpart plugin. >> This way the amount of compiled files should be reduced as well. >> I attached the patch for that, can you please look at it? >>> >>>> I would prefer to backport the patch too, since it is rather valuable for >>>> us. >>> >>> Generally speaking, only small bug-fixing patches should be >>> backported. Plus as you mentioned yourself, there are still some >>> problems to iron out on Windows. >> >> If I make more indepth changes this would be of course stupid (the other >> changes wouldn't have changed anything for Linux). That's why I would apply >> all changes only to trunk. >> >>> >>> Regards, >>> Robert. >> >> Thanks so far, >> Patrick >>> >> >> >> Ok, just to give you an impression, here is a screenshot of konsole: >> http://imagebin.ca/view/LoZdwOM.html >> >> >> -- >> web: http://windows.kde.org >> mailing list: kde-windows@kde.org >> irc: #kde-windows (irc.freenode.net) >> >> _______________________________________________ >> konsole-devel mailing list >> konsole-devel@kde.org >> https://mail.kde.org/mailman/listinfo/konsole-devel >> >> > _______________________________________________ > konsole-devel mailing list > konsole-devel@kde.org > https://mail.kde.org/mailman/listinfo/konsole-devel > _______________________________________________ konsole-devel mailing list konsole-devel@kde.org https://mail.kde.org/mailman/listinfo/konsole-devel