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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to