[PATCH] D21814: clang-rename: support multiple renames with one invocation

[Kirill Bobyrev via cfe-commits]

omtcyf0 added a comment.

Miklos, thanks for the feedback!

Hm, I'm not sure about a) and b) camps here. I think we can have both. It may be that I haven't looked too much into the code or I am missing something, but so far both integration and cross-TU analysis seem OK together in one tool as for me.

So far, integration (in it's very very simple way) is very straightforward: we just have a binary and we call it via passing two parameters (offset and new-name). This doesn't require neither changes in the tool as it is nor any difficult tweaks.

As for cross-TU stuff, why can't it be here, too? We'll still need it at some point.

Creating `clang-multi-rename` doesn't seem right at all - there are already too many tools which some users don't understand. If the command line interface with this feature is a useful thing to have, then we should definitely just add it here. One issue I see is that I think it won't be useful to most users, but a) I can be wrong b) It's not the main one :)

Probably my main point is: **let's not bring too much features into the interface we don't know anything about yet**. It feels right to me to ensure that //the tool actually works correctly//. Because it does not at the moment (see http://reviews.llvm.org/D22102 XFAIL test for example).

After we do that, we will have the feedback from the community, because they will try the first version, which actually works. We might get at least few people just trying it at least and saying whether for example it's not usable at all in the editor and that they would like an extended CLI. I'll be all for the changes similar to these proposed by you then!

In my opinion we should (right now) only support a single use scenario: `clang-rename -offset=[OFFSET] -new-name=[NAME]`. Not only because we would keep things easy (which I'd honestly be very happy about, but, I know, people hate me for that :D), but because when we get our first users, chances of them finding some configuration of parameters which breaks than simples single scenario is very high, not talking about the multiple scenarios you're proposing. Why would we need many ways to interact with an incorrect tool? We should probably have a single way to interact with a completely correct tool.

At the end I think we can both be happy if we separate the CLI and the renaming interface completely. The editors can interact with a C++ Library then and `clang-rename` would become a (probably) sophisticated command line interface to that library, only dealing with user-library interaction thingie. We can't have both at the moment, of that I am certain.

My proposal is to:

- Improve Vim interface just a little, I think I can do that tomorrow.
- Write a letter to the cfe-dev, explaining that we want `clang-rename` to become useful and that we want their feedback. I'll send it tomorrow, right after I'll be done with the Vim interface slight improvements. We'll probably get more interesting ideas and, most importantly, at least few first users, who will help us to understand what they need. Discussions on reviews get lost and forgotten, cfe-dev is always there and living.
- Get a proper test set for clang-rename. Really. Because now it's not tested at all. Fill in bugs (hopefully the guys who will try it at least will help), get suggestions, get opinions. Probably make a list/plan of them.
- Address the issues and then think of the additional stuff we can add (like separating the interface and routine and all that).

As I'm going to invest very much time into the project I hope we can do that!

I'll be very happy to hear more opinions from the others both here and in the cfe-dev list after I send the message!

@klimek, @bkramer, what do you think of this?


http://reviews.llvm.org/D21814