On 12/08/2010 05:15 PM, ext Panagiotis Papadopoulos wrote:
> The second patch from “our” Student Furkan has landed:
> Add a way to copy URLs from the Network Analyzer Panel
>
> http://socghop.appspot.com/gci/task/show/google/gci2010/kde/t129158022937
>
> Is it ok, like it is?
From a quick look:
It would be nice to sort the includes by alphabetical order (like for
WebKit). :)
NetworkAnalyzer::contextMenuEvent() mixes spaces and tabs for the
indentation.
In NetworkAnalyzer::contextMenuEvent(), the parent of the KMenu is
messing things up. QObjects created on the stack should not have parents.
+ KAction *copy;
+ copy = new KAction(KIcon("edit-copy"),i18n("Copy URL"), this);
Could be:
KAction *const copy = new KAction(KIcon("edit-copy"),i18n("Copy URL"),
this);
(Just a suggestion, some knows how much I like "const" :)).
In NetworkAnalyzer::contextMenuEvent(), the connect() misses spaces
after the comas to follow the coding style.
I am not sure how this connection works:
connect( _requestList, SIGNAL(customContextMenuRequested(QPoint)), this,
SLOT(contextMenuEvent(QContextMenuEvent*))) ;
The arguments seem incompatible to me.
cheers,
Benjamin
_______________________________________________
rekonq mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/rekonq
