[prev in list] [next in list] [prev in thread] [next in thread]
List: konsole-devel
Subject: Re: [Konsole-devel] [PATCH] Windows port
From: Andrius da Costa Ribas <andriusmao () gmail ! com>
Date: 2009-05-15 11:04:46
Message-ID: a5602ba00905150404v7fcb87banb5174f54ab35974a () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Any updates about this topic?
2009/2/10 Andrius da Costa Ribas <andriusmao@gmail.com>
> Is it possible to include a preview binary in the installer?
>
> 2009/2/1 Robert Knight <robertknight@gmail.com>:
> > 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 <ps_ml@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@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
> >
>
[Attachment #5 (text/html)]
Any updates about this topic?<br><br><div class="gmail_quote">2009/2/10 Andrius da \
Costa Ribas <span dir="ltr"><<a \
href="mailto:andriusmao@gmail.com">andriusmao@gmail.com</a>></span><br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex;">
Is it possible to include a preview binary in the installer?<br>
<br>
2009/2/1 Robert Knight <<a \
href="mailto:robertknight@gmail.com">robertknight@gmail.com</a>>:<br> \
<div><div></div><div class="h5">> Hi Patrick,<br> ><br>
> Seems sensible to me but it is probably best to discuss issues around<br>
> dividing the code into libraries on <a \
href="mailto:kde-devel@kde.org">kde-devel@kde.org</a><br> > where there are \
developers who understand this better than I do.<br> ><br>
> Regards,<br>
> Robert.<br>
><br>
><br>
> 2009/2/1 Patrick Spendrin <<a \
href="mailto:ps_ml@gmx.de">ps_ml@gmx.de</a>>:<br> >> Robert Knight \
schrieb:<br> >>><br>
>>> Hi,<br>
>>><br>
>>> Thank-you for the patch. I prefer platform specific #if-defs to be<br>
>>> kept to a minimum. If you are writing code which would compile on \
all<br> >>> platforms but should only be executed on some of them, eg:<br>
>>><br>
>>> +#ifndef Q_WS_WIN<br>
>>> setPtyChannels(KPtyProcess::AllChannels);<br>
>>> +#else<br>
>>> + setOutputChannelMode(SeparateChannels);<br>
>>> +#endif<br>
>><br>
>> Especially this code doesn't compile here ;-) (due to missing \
KPtyProcess)<br> >> and most of the code used for real terminal interaction \
doesn't compile<br> >> here, so it surely won't work without ifdefs \
(but you're right, we can<br> >> minimize them).<br>
>> A patch for that needs me some more time though.<br>
>>><br>
>>> Then it would be better to put the platform specific code in a<br>
>>> sub-class and call it via a virtual function. This is easier to \
read<br> >>> and compile-time breakages in code for other platforms are \
detected<br> >>> sooner - see the ProcessInfo class implementations for \
Linux and<br> >>> Solaris. Things are slightly more complex here since you \
want to use<br> >>> KProcess as the base class on Windows and KPtyProcess on \
Unix. That<br> >>> could be fixed by making Konsole::Pty not inherit \
directly from<br> >>> KProcess but have a KProcess as a field internally and \
provide a<br> >>> virtual process() accessor.<br>
>>><br>
>>> +#ifndef Q_WS_WIN<br>
>>> class KDE_EXPORT Profile : public QSharedData<br>
>>> +#else<br>
>>> +class Profile : public QSharedData<br>
>>> +#endif<br>
>>><br>
>>> Why is KDE_EXPORT being removed here on Windows? If it is required,<br>
>>> the KDE_EXPORT macro should be replaced with something else (eg.<br>
>>> KONSOLE_TEST_EXPORT) which is #defined appropriately depending on \
the<br> >>> platform.<br>
>><br>
>> I am currently trying to find a way through this.<br>
>> The small problem is that there is a difference between a kdeinit \
executable<br> >> under Windows and under Linux. the first is a normal \
application, whereas<br> >> the second is more of the library kind. This also \
makes linking to the<br> >> kdeinit executable possible under Linux (and that \
way also exporting from<br> >> it) whereas this doesn't work under \
windows.<br> >><br>
>> Edit: I decided to make up a libkonsoleprivate instead which is linked \
by<br> >> the tests, the kdeinit executable and the kpart plugin.<br>
>> This way the amount of compiled files should be reduced as well.<br>
>> I attached the patch for that, can you please look at it?<br>
>>><br>
>>>> I would prefer to backport the patch too, since it is rather \
valuable for<br> >>>> us.<br>
>>><br>
>>> Generally speaking, only small bug-fixing patches should be<br>
>>> backported. Plus as you mentioned yourself, there are still some<br>
>>> problems to iron out on Windows.<br>
>><br>
>> If I make more indepth changes this would be of course stupid (the other<br>
>> changes wouldn't have changed anything for Linux). That's why I \
would apply<br> >> all changes only to trunk.<br>
>><br>
>>><br>
>>> Regards,<br>
>>> Robert.<br>
>><br>
>> Thanks so far,<br>
>> Patrick<br>
>>><br>
>><br>
>><br>
>> Ok, just to give you an impression, here is a screenshot of konsole:<br>
>> <a href="http://imagebin.ca/view/LoZdwOM.html" \
target="_blank">http://imagebin.ca/view/LoZdwOM.html</a><br> >><br>
>><br>
>> --<br>
>> web: <a href="http://windows.kde.org" \
target="_blank">http://windows.kde.org</a><br> >> mailing list: <a \
href="mailto:kde-windows@kde.org">kde-windows@kde.org</a><br> >> irc: \
#kde-windows (<a href="http://irc.freenode.net" \
target="_blank">irc.freenode.net</a>)<br> >><br>
>> _______________________________________________<br>
>> konsole-devel mailing list<br>
>> <a href="mailto:konsole-devel@kde.org">konsole-devel@kde.org</a><br>
>> <a href="https://mail.kde.org/mailman/listinfo/konsole-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/konsole-devel</a><br> \
>><br> >><br>
> _______________________________________________<br>
> konsole-devel mailing list<br>
> <a href="mailto:konsole-devel@kde.org">konsole-devel@kde.org</a><br>
> <a href="https://mail.kde.org/mailman/listinfo/konsole-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/konsole-devel</a><br> ><br>
</div></div></blockquote></div><br>
_______________________________________________
konsole-devel mailing list
konsole-devel@kde.org
https://mail.kde.org/mailman/listinfo/konsole-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic