Review Request 115383: Reuse code in Refactor widget. Changes in kdevplatform

Sergey Kalinichev kalinichev.so.0 at gmail.com
Thu Jan 30 08:45:48 UTC 2014



> On Jan. 29, 2014, 8:36 p.m., Milian Wolff wrote:
> > language/codegen/basicrefactoring.h, line 60
> > <https://git.reviewboard.kde.org/r/115383/diff/1/?file=241014#file241014line60>
> >
> >     put brace on newline

BTW is there some ready to use source formatter for that? Because integrated in kdevelop doesn't do it automatically (I set up KdeLibs style, but seems like it's a little bit different from what you want...)
How do you do it?


> On Jan. 29, 2014, 8:36 p.m., Milian Wolff wrote:
> > language/duchain/navigation/useswidget.h, line 157
> > <https://git.reviewboard.kde.org/r/115383/diff/1/?file=241016#file241016line157>
> >
> >     this should get a big fat TODO comment that it should be ported to use shared ptr around m_collector - the api right now is really ugly imo - could you clean this up in a following patch please?

I've fixed the api more or less. Still the problem is that UsesWidgetCollector is too highly coupled to gui.(it neither collector nor it wigdet). I wanted to decouple it, but it requires some changes to BasicRefactoringCollector and transferring it to some more general place e.g. to usescollector.h. Still maybe I simply don't understand how components suppose to communicate with each other...
So instead I check that m_widget is still visible, if not do nothing inside UsesWidgetCollector, but BasicRefactoringCollector will continue collecting uses anyway.


> On Jan. 29, 2014, 8:36 p.m., Milian Wolff wrote:
> > language/codegen/basicrefactoring.cpp, line 189
> > <https://git.reviewboard.kde.org/r/115383/diff/1/?file=241015#file241015line189>
> >
> >     moving code around in the same patch makes it hard to review... please keep this in mind for future changes and submit first a cleanup patch, then the smaller patch which adds the functionality.

Hmmm, I don't remember moving the code around. Seems like reviewboard's diff tool is not smart enough to distinguish code moving from code inserting/refactoring big function into little ones.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115383/#review48558
-----------------------------------------------------------


On Jan. 30, 2014, 12:42 p.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115383/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2014, 12:42 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Refactore code out a little bit so that language plugins can reuse even more code if they want to reimplement startInteractiveRename function.
> 
> 
> Diffs
> -----
> 
>   language/duchain/navigation/useswidget.h f52d868 
>   language/codegen/basicrefactoring.cpp d5e8c67 
>   language/codegen/basicrefactoring.h 06606a5 
>   language/duchain/navigation/useswidget.cpp 2fd0bd9 
> 
> Diff: https://git.reviewboard.kde.org/r/115383/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140130/cabd7cf1/attachment.html>


More information about the KDevelop-devel mailing list