I've just submitted artifact 2808711 (https://sourceforge.net/tracker/? func=detail&aid=2808711&group_id=6488&atid=306488) with patch that
adds service menus. It uses -D__LIBKONQ__ flag just like it was using.
Please tell me what you think about it. It works well for me, that's a
functionality I've missed the most in Krusader for KDE4. I hope it can
find its way into trunk.
Patch was done against SourceForge's trunk repository.
> I've just submitted artifact 2808711 (https://sourceforge.net/tracker/? > func=detail&aid=2808711&group_id=6488&atid=306488) with patch that
> adds service menus. It uses -D__LIBKONQ__ flag just like it was using.
> Please tell me what you think about it. It works well for me, that's a
> functionality I've missed the most in Krusader for KDE4. I hope it can
> find its way into trunk.
> Patch was done against SourceForge's trunk repository.
some comments: - in cmake files, you should use something like find_package(LibKonq REQUIRED) and then use LIBKONQ_FOUND - if some parts of the code is known deprecated for KDE4.3, it should be fixed now using KDE_VERSION ;)
On Jun 19, 9:25 am, Fathi Boudra <fbou...@gmail.com> wrote:
> some comments:
> - in cmake files, you should use something like find_package(LibKonq
> REQUIRED) and then use LIBKONQ_FOUND
dolphin and kdepasswd depends on libkonq so I thought it's one of
basic kde libraries therefore there is no need to look for it. Should
I look for it anyway? (you don't look for other ${KDE_XXX} libraries).
> - if some parts of the code is known deprecated for KDE4.3, it should
> be fixed now using KDE_VERSION ;)
I use stable KDE 4.2.2, I just saw that two method signatures are
changed in KDE's trunk so I put comments on it. Should I either: 1)
leave this patch as it is (solution for KDE 4.3 is in comments), 2)
move to KDE4.3 beta and make appropiate changes, 3) get KDE from trunk
and submit a working KDE4.3 patch against Krusader's SF repository, 4)
get KDE from trunk and submit a working 4.3 patch against Krusader's
KDE extragear repository?
> On Jun 19, 9:25 am, Fathi Boudra <fbou...@gmail.com> wrote: >> some comments: >> - in cmake files, you should use something like find_package(LibKonq >> REQUIRED) and then use LIBKONQ_FOUND
> dolphin and kdepasswd depends on libkonq so I thought it's one of > basic kde libraries therefore there is no need to look for it.
because they are in kdebase so they don't check it. take a look to third application like kdiff3 or konq-plugins.
> I look for it anyway? (you don't look for other ${KDE_XXX} libraries).
imho, yes. It's the proper way.
>> - if some parts of the code is known deprecated for KDE4.3, it should >> be fixed now using KDE_VERSION ;)
> I use stable KDE 4.2.2, I just saw that two method signatures are > changed in KDE's trunk so I put comments on it. Should I either: 1) > leave this patch as it is (solution for KDE 4.3 is in comments), 2) > move to KDE4.3 beta and make appropiate changes, 3) get KDE from trunk > and submit a working KDE4.3 patch against Krusader's SF repository, 4) > get KDE from trunk and submit a working 4.3 patch against Krusader's > KDE extragear repository?
you can postpone the fix and add a TODO tag as we're aware of the deprecated functions. I (or someone else) can fix it as soon as csaba ack the patch.
> > I look for it anyway? (you don't look for other ${KDE_XXX} libraries).
> imho, yes. It's the proper way.
> you can postpone the fix and add a TODO tag as we're aware of the
> deprecated functions.
> I (or someone else) can fix it as soon as csaba ack the patch.
I've uploaded a second version of a patch. It has proper CMake usage
of find_package(LibKonq) and TODO marks for KDE4.3. I think it's
acceptable now.
I think, that KDE_VERSION is acceptable only if we use new features
which are not yet available in KDE 4.2. The best is not to add any
deprecated code, but wait for KDE 4.3 if possible.
Fathi, if KDE4 doesn't like the KDE3 support libs, you can drop
them (just remove the deprecated KrDetailedView, KrBriefView and their
references from KrViewFactory).
Krusader should compile and work well without using the support libs.
>> On Jun 19, 9:25 am, Fathi Boudra <fbou...@gmail.com> wrote:
>>> some comments:
>>> - in cmake files, you should use something like find_package(LibKonq
>>> REQUIRED) and then use LIBKONQ_FOUND
>> dolphin and kdepasswd depends on libkonq so I thought it's one of
>> basic kde libraries therefore there is no need to look for it.
> because they are in kdebase so they don't check it.
> take a look to third application like kdiff3 or konq-plugins.
>> I look for it anyway? (you don't look for other ${KDE_XXX} libraries).
> imho, yes. It's the proper way.
>>> - if some parts of the code is known deprecated for KDE4.3, it should
>>> be fixed now using KDE_VERSION ;)
>> I use stable KDE 4.2.2, I just saw that two method signatures are
>> changed in KDE's trunk so I put comments on it. Should I either: 1)
>> leave this patch as it is (solution for KDE 4.3 is in comments), 2)
>> move to KDE4.3 beta and make appropiate changes, 3) get KDE from trunk
>> and submit a working KDE4.3 patch against Krusader's SF repository, 4)
>> get KDE from trunk and submit a working 4.3 patch against Krusader's
>> KDE extragear repository?
> you can postpone the fix and add a TODO tag as we're aware of the
> deprecated functions.
> I (or someone else) can fix it as soon as csaba ack the patch.
> I think, that KDE_VERSION is acceptable only if we use new features > which are not yet available in KDE 4.2. The best is not to add any > deprecated code, but wait for KDE 4.3 if possible.
most users still use 4.2 series provided by distributions. the "deprecated" code is only for bleeding edge users. imho, we should support both 4.2 and 4.3 users.
> Fathi, if KDE4 doesn't like the KDE3 support libs, you can drop > them (just remove the deprecated KrDetailedView, KrBriefView and their > references from KrViewFactory).
> Krusader should compile and work well without using the support libs.
KDE4 works with KDE3 support libs but current hack is a bit ugly (QT3_SUPPORT defines). if KrDetailedView and KrBriefView are deprecated, I'll be more than happy to drop it including support libs linkage.
> I think, that KDE_VERSION is acceptable only if we use new features > which are not yet available in KDE 4.2. The best is not to add any > deprecated code, but wait for KDE 4.3 if possible.
also, the 2nd submitted patch doesn't use KDE_VERSION but add comments with required changes marked as TODO. as you suggested, keeping code for KDE 4.2 only is fine.
> I removed the old detailed + brief views. The new interviews have to > completely replace the old ones. If there's any issue with them, please > tell me.
> Fathi,
> Krusader doesn't use KDE3 support libs any more. I've checked with ldd, > and there was no such dependency.
>> I think, that KDE_VERSION is acceptable only if we use new features
>> which are not yet available in KDE 4.2. The best is not to add any
>> deprecated code, but wait for KDE 4.3 if possible.
> also, the 2nd submitted patch doesn't use KDE_VERSION
> but add comments with required changes marked as TODO.
> as you suggested, keeping code for KDE 4.2 only is fine.
On 20 Cze, 12:41, Karai Csaba <cska...@freemail.hu> wrote:
> Added the service menu PATCH to SVN.
Thank you :)
> I've modified only the CMake files, as Krusader didn't compile when
> libkonq library was missing.
I tested it with and without libkonq and both were working for me, but
maybe I've missed something. I'm glad you took care of testing and
repairing it for me.
I've also closed my submitted artifact as Accepted on sf.net. BTW.
there is a lot of submitted bugs and patches opened :o probably half
of them is outdated, but still they remain on tracker.
|
