hokein 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";
     }
----------------
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";


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

Reply via email to