Review Request 124300: Change signature
Maciej Poleski
d82ks8djf82msd83hf8sc02lqb5gh5 at gmail.com
Sun Jul 26 12:08:49 UTC 2015
> On Lip 25, 2015, 3:28 po południu, Milian Wolff wrote:
> > hm have you ever tried out oldcpp's "change signature" feature? That one was _much_ better useability wise. You simply edited the signature, and an assistant asked you whether you want to copy the changes to the corresponding define/declaration. That would be much better to copy. Having an editor dialog like you propose here is going to be cumbersome and I don't see myself using that I'm afraid to admit.
> >
> > What do you say? Can you reuse this code to implement the oldcpp behavior for changing a signature?
>
> Maciej Poleski wrote:
> You mean "adapt signature" (after change of declaration in source code)?
> It is something different. I don't know how to track changes in source code so gracefully, but the main issue here is that i wan't to track changes very precisely so that I preserve as much original information as possible. The most important is not changing signature (in modern code bases we have (one forward declaration +) one definition - at most 2 entities), but adapting uses (arbitrary number of function calls and references). If we only change order of parameters (or remove some) - we can automatically adapt all uses so that code remians semantically equal (change of function name doesn't matter - we always can handle that). Only introduction of new parameters breaks code. This a bit strange dialog (QTableView + 4 QToolButton to be precise) has one important task: collect information in form of binding from original signature to new one.
> As I previously mentioned I'm not a good GUI designer. I can give features, usabilitity is always an issue. But I can hardly find necessary informations in "adapt signature" style (even if i forget that i don't know how to handle direct changes in source code (how to use Clang to handle them, to be precise)).
>
> Milian Wolff wrote:
> Ah ok, and the adapt signature assistant is of course already implemented as well, my bad. Question is, if we could eventually reuse this code from there as well. Anyhow, I'll review the code now, and consider my statement above invalidated.
Almost yes. We need `DeclarationComparator`. This object is generated from `const FunctionDecl*`. We would need to get this AST node somehow. Then it shouldn't be difficult do generate info pack. After that, action can be invoked on `RefactoringContext` without use of `ChangeSignatureRefactoring` object.
- Maciej
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124300/#review82934
-----------------------------------------------------------
On Lip 13, 2015, 11:34 po południu, Maciej Poleski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124300/
> -----------------------------------------------------------
>
> (Updated Lip 13, 2015, 11:34 po południu)
>
>
> Review request for KDevelop.
>
>
> Repository: kdev-clang
>
>
> Description
> -------
>
> Change signature refactoring
>
> I decided not to implement rename function refactoring. Change signature will provide this functionality.
> We can rename all functions except constructors, destructors, conversion operators, operators.
> Operators must preserve number of parameters (imagine 2+2 and operator+).
>
> This functionality allows to:
> - change return type
> - change function name
> - change parameter types (preserving identity, tracking of this is not perfect - we lose this information)
> - change order of parameters
> - introduce new parameters
> - remove existing parameters
>
> Somewhere here should be screenshoot of this dialog...
>
>
> Diffs
> -----
>
> refactoring/CMakeLists.txt PRE-CREATION
> refactoring/changesignaturedialog.h PRE-CREATION
> refactoring/changesignaturedialog.cpp PRE-CREATION
> refactoring/changesignaturedialog.ui PRE-CREATION
> refactoring/changesignaturerefactoring.h PRE-CREATION
> refactoring/changesignaturerefactoring.cpp PRE-CREATION
> refactoring/changesignaturerefactoringchangepack.h PRE-CREATION
> refactoring/changesignaturerefactoringchangepack.cpp PRE-CREATION
> refactoring/changesignaturerefactoringinfopack.h PRE-CREATION
> refactoring/changesignaturerefactoringinfopack.cpp PRE-CREATION
> refactoring/debug.h PRE-CREATION
> refactoring/refactoring.h PRE-CREATION
> refactoring/refactoring.cpp PRE-CREATION
> refactoring/refactoringmanager.cpp PRE-CREATION
> refactoring/utils.h PRE-CREATION
> refactoring/utils.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/124300/diff/
>
>
> Testing
> -------
>
> Manual testing of GUI. No transformation tests yet...
>
>
> File Attachments
> ----------------
>
> snapshot
> https://git.reviewboard.kde.org/media/uploaded/files/2015/07/08/e8a8db09-92f9-4fb6-ac4a-3ab2a5051033__snapshot1.png
>
>
> Thanks,
>
> Maciej Poleski
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150726/c5a79d7e/attachment.html>
More information about the KDevelop-devel
mailing list