[Konsole-devel] [PATCH] Windows port

[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 at kde.org
irc:                 #kde-windows (irc.freenode.net)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0140-libkonsoleprivate-without-ifdefs.diff
Type: text/x-patch
Size: 12504 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20090201/7a765838/attachment.diff>