[Konsole-devel] [PATCH] Windows port

[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 at kde.org
where there are developers who understand this better than I do.

Regards,
Robert.


2009/2/1 Patrick Spendrin <ps_ml at gmx.de>:
> 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)
>
> _______________________________________________
> konsole-devel mailing list
> konsole-devel at kde.org
> https://mail.kde.org/mailman/listinfo/konsole-devel
>
>