From konsole-devel Fri May 15 11:04:46 2009 From: Andrius da Costa Ribas Date: Fri, 15 May 2009 11:04:46 +0000 To: konsole-devel Subject: Re: [Konsole-devel] [PATCH] Windows port Message-Id: X-MARC-Message: https://marc.info/?l=konsole-devel&m=124238557100518 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0072135212==" --===============0072135212== Content-Type: multipart/alternative; boundary=001636e0b5e3c3969f0469f16a0b --001636e0b5e3c3969f0469f16a0b Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Any updates about this topic? 2009/2/10 Andrius da Costa Ribas > 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 > > > --001636e0b5e3c3969f0469f16a0b Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Any updates about this topic?

2009/2/10 A= ndrius da Costa Ribas <andriusmao@gmail.com>
Is it possible to include a preview binary in the installer?

2009/2/1 Robert Knight <robert= knight@gmail.com>:
> Hi Patrick,
>
> Seems sensible to me but it is probably best to discuss issues around<= br> > 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 <ps_ml@gm= x.de>:
>> Robert Knight schrieb:
>>>
>>> Hi,
>>>
>>> Thank-you for the patch. =A0I prefer platform specific #if-def= s to be
>>> kept to a minimum. =A0If you are writing code which would comp= ile on all
>>> platforms but should only be executed on some of them, eg:
>>>
>>> +#ifndef Q_WS_WIN
>>> =A0 setPtyChannels(KPtyProcess::AllChannels);
>>> +#else
>>> + =A0setOutputChannelMode(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 r= ight, 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. =A0This is easie= r to read
>>> and compile-time breakages in code for other platforms are det= ected
>>> sooner - see the ProcessInfo class implementations for Linux a= nd
>>> Solaris. =A0Things are slightly more complex here since you wa= nt to use
>>> KProcess as the base class on Windows and KPtyProcess on Unix.= =A0That
>>> could be fixed by making Konsole::Pty not inherit directly fro= m
>>> KProcess but have a KProcess as a field internally and provide= a
>>> virtual process() accessor.
>>>
>>> +#ifndef Q_WS_WIN
>>> =A0class KDE_EXPORT Profile : public QSharedData
>>> +#else
>>> +class Profile : public QSharedData
>>> +#endif
>>>
>>> Why is KDE_EXPORT being removed here on Windows? =A0If it is r= equired,
>>> the KDE_EXPORT macro should be replaced with something else (e= g.
>>> 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 exporti= ng from
>> it) whereas this doesn't work under windows.
>>
>> Edit: I decided to make up a libkonsoleprivate instead which is li= nked 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 rath= er valuable for
>>>> us.
>>>
>>> Generally speaking, only small bug-fixing patches should be >>> backported. =A0Plus 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 konsol= e:
>> http://imagebin.ca/view/LoZdwOM.html
>>
>>
>> --
>> web: =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 http://windows.kde.org
>> mailing list: =A0 =A0 =A0 =A0kde-windows@kde.org
>> irc: =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 #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
>

--001636e0b5e3c3969f0469f16a0b-- --===============0072135212== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ konsole-devel mailing list konsole-devel@kde.org https://mail.kde.org/mailman/listinfo/konsole-devel --===============0072135212==--