PATCH: Append servicemenus to right-click file context menu

[Tomasz Kalkosiński]

Hello

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.

Greetings,
Tomasz Kalkosiński

[Karai Csaba]

Thanks, Tomasz

We'll check your patch and add it to the trunk.
I also think, that it's an important feature, but I thought it's not
possible to implement.

:-)

Csaba

Tomasz Kalkosiński írta:

[Fathi Boudra]

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 ;)

cheers,

Fathi

[Tomasz Kalkosiński]

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?

Greetings,
Tomasz Kalkosiński

[Fathi Boudra]

> 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.

thanks for the patch.

[Tomasz Kalkosiński]

> > 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 watched trunk's dolphin context menu as a KDE4.3 example. Take a
look at dolphin menu building here:
http://websvn.kde.org/trunk/KDE/kdebase/apps/dolphin/src/dolphincontextmenu.cpp?revision=973279&view=markup
.

Greetings,
Tomasz Kalkosiński

[Karai Csaba]

Hi,

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.

Thanks,

Csaba



Fathi Boudra írta:

[Fathi Boudra]

> 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.

cheers,

Fathi

[Fathi Boudra]

> 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.

[Karai Csaba]

Hi,

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.

:)

Csaba

[Fathi Boudra]

great \o/
svn up :)

thanks.

[Karai Csaba]

Added the service menu PATCH to SVN.
I've modified only the CMake files, as Krusader didn't compile when
libkonq library was missing.

Later adding the KDE_VERSION macro would be useful, but as I have no KDE
4.3, I couldn't test whether it works well.

Thanks,

Csaba

Fathi Boudra írta:

[Tomasz Kalkosiński]

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.

Greetings,
Tomasz Kalkosiński