kbobyrev marked an inline comment as not done.
kbobyrev added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:218
+ case ReasonToReject::SameName:
+ return "new name should differ from the old name";
}
----------------
hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > returning an error seems to be an interesting idea, thinking more about
> > > that, it might be better than just returning an empty workspaceEdit
> > > silently without noticing user.
> > >
> > We don't know what the user's intent was here - it's at least somewhat
> > likely they changed their mind about the operation but hit "enter" instead
> > of "escape". So a message describing the situation "new name is the same as
> > the old name" would be more appropriate than suggesting a correction.
> >
> > But I'm wondering whether this error message actually helps (vs
> > "succeeding" with no edits). Is it actionable? What's the scenario where
> > the user:
> > a) doesn't already know that the name is the same, and
> > b) will take some action as a result (rather than leave the name unchanged)
> >
> > a message describing the situation "new name is the same as the old name"
> > would be more appropriate than suggesting a correction.
>
> SG.
>
> In an ideal world, I'd expect we emit a non-error message to inform that
> there is nothing happen for the rename, but LSP doesn't have such option :(
>
> The behavior of editors are varied on this case, e.g.
> - VSCode just succeeds with no edits;
> - our internal editor emits a message like "nothing to rename";
> But I'm wondering whether this error message actually helps (vs "succeeding"
> with no edits). Is it actionable? What's the scenario where the user:
> a) doesn't already know that the name is the same, and
> b) will take some action as a result (rather than leave the name unchanged)
I can understand this point of view, but I'm a bit sceptical of the "fail"
silently approach. The "actionable" for user would be to not type in the same
name as before :)
Also, user might wanted to rename the symbol to something slightly different
(e.g. fix typo) and then have a typo in new name, too. I think there is value
in telling the user that there was no rename because the name is the same - I
don't think it hurts them in any way. We're just being explicit in what
happened/didn't happen. I think "new name = old name" can be a reasonable
"error" scenario and I think there is a definite value in being explicit about
what we do or decide not to do.
My point is that I can't see a scenario when user actually wants to rename the
symbol deliberately using the same name and I think this should, in fact, be an
error to do so.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91134/new/
https://reviews.llvm.org/D91134
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
