[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwrite-devel
Subject:    Re: [PATCH] Icon-Inserter Plugin
From:       "Jonathan =?iso-8859-1?q?Schmidt-Domin=E9_-?= Developer" <devel () the-user ! org>
Date:       2009-10-21 20:35:28
Message-ID: 200910212235.29543.devel () the-user ! org
[Download RAW message or body]

Hi!

> Please commit to playground/devtools, and continue improving it, if
> neccessary.
I've not yet a SVN-account.

> I've yet to test your code, but I find the "adding to context-menu" code
>  kinda strange. Could be that I misunderstand the API, but I'd think just
>  connecting to the signal aboutToShowContextmenu and _always_ adding the
>  action should be correct. But well, I never did that, so I'll have to try
>  it out.
Well, it works. addActions adds the action permanently. But I had to work 
around the issue that the plugin is loaded before the context-menu is created 
by Kate.
 
> Other than that just some code style questions, which are definitly
>  personal taste. I'd personally never use "== 0" when checking for a
>  pointer, but just "!".
I personally prefer "== 0", seems to be clearer for me.
>  Also I'd opt for pushing all declartaions to .cpp
>  and leaving the .h with definitions only. I doubt making createAction
>  inline is noticable and required.
For a one-class-plugin, independent from everything, these issues are totally 
unimportant. :)

Jonathan

------------------------
Automatisch eingefügte Signatur:
Es lebe die Freiheit!
Stoppt den Gebrauch proprietärer Software!
Operating System: GNU/Linux
Kernel: Linux 2.6.31-ARCH
Distribution: Arch Linux
Qt: 4.5.1
KDE: 4.3.71 (KDE 4.3.71 (KDE 4.4 >= 20091007))
KMail: 1.12.90

_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel

[prev in list] [next in list] [prev in thread] [next in thread]