-
Notifications
You must be signed in to change notification settings - Fork 1.5k
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Please add renaming #296
Comments
I agree. |
@tinganho, Renaming is a feature that we consider part of a much larger work-item to implement more intelligent auto-complete (aka Intellisense(tm) in the Visual Studio world) for our extension. We hope to roll that out in the coming months. |
I agree too. Is there some roadmap or state of the rename symbol feature? The greazer's comment is old and doesn't make sense now, when intelligent auto-complete aka Intellisense is already in VS Code using C/C++ extension. Just the rename symbol is missing :( How is it going? |
Actually, the IntelliSenseEngine features we have so far are just error squiggles and hover (quick info). We are currently working on getting auto-complete added (it's currently using the old implementation that has issues, such as no local variables and too many invalid results). Unfortunately, we don't have a roadmap for when rename will be implemented. |
Now that reference highlighting is implemented in the September cpptools update, it seems quite awkward that Change All Occurrences changes a different set of occurrences to what are highlighted. Is the rename feature the intended way to do that in other languages? It almost feels like a bug in VSCode now that this inconsistency is created. |
Hmm... I never thought about that. Reference highlighting only ever applies to a single file at a time but rename works across the entire project so I never made that connection. Unfortunately, "rename" is correlated to "find all references" - which we have not implemented yet - so the addition of reference highlighting doesn't get us much closer to the implementation of rename. |
Hi guys - the extension works great! - just wondering if there are any updates on this issue? |
@dprandle There are no updates on renaming. When we decide to start working on renaming it will be added to a milestone. It looks like our 5th top up-voted issue: https://github.com/Microsoft/vscode-cpptools/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc . |
@sean-mcmanus @dprandle I want to add, that from my point of view, it's highly coupled with 1st top up-voted issue (if you can find all references, then you just rename them all). I know, it's a bit different, but solving that moves it forward a lot I think. |
@salda Yeah, most of the top features are dependent on the multiple translation unit IntelliSense component, which hasn't been ported from VS 2017 yet. We want to fix the major problems with configuration for single translation unit IntelliSense before starting work on that, e.g. recursive includePath, compilerPath for WSL/cl.exe (better guessing/defaults), global settings, etc. |
When you do bring over intellisense please base it on libclang. In the current intellisense (in VisualStudio 2017) I have code that compiles correctly (I am using the clang tools extension with clang 6.0: https://marketplace.visualstudio.com/items?itemName=LLVMExtensions.llvm-toolchain) but according to intellisense the source has errors, because (I presume) it uses a faulty msvc front end (it is conformant c++17 code that will not compile with the current trunk of msvc). It is annoying to have red squiggles through source that actually compiles without error or warnings. |
I think it's better to just say not to use VS2017's compiler. |
I'm not using msvc, I am using clang, however, intellisense still uses the msvc front end apparently, which leads to a disconnect between what is actually compilable and what intellisense thinks is compilable. If intellisense is based on libclang (for vscode) then everything will be good, if it is based on msvc then it will be bad. I can live with the issue in Visual Studio, but I would like to not have the same issue in vscode when it gets full intellisense (i.e. intellisense == good, intellisense based on msvc == not so good). |
@rtosman, our IntelliSense engine has clang and gcc compatibility modes. If you are seeing incorrect error squiggles with those modes, please open a new issue and we'll investigate getting them fixed. This issue is about support for renaming. |
@bobbrow , I was referring to what visual studio does, in the context of this discussion with reference to the comment from @sean-mcmanus "Yeah, most of the top features are dependent on the multiple translation unit IntelliSense component, which hasn't been ported from VS 2017 yet." I have compatibility mode in vscode set to "clang-x64" and it works fine within the limitations discussed. My point was that the intellisense that visual studio has does not correctly operate with clang, which I presume is because the intellisense engine in visual studio is based on msvc (I presume this to be the case, since the errors it "squiggles" are the same errors I would get if I did compile the code with msvc. The errors in question are clearly out-of-scope for the current level of intellisense support in vscode and I do not see squiggles with the same code in vscode. I was simply asking that when you do bring over full blown intellisense to vscode (part of getting renaming working) to please base it on libclang so that it doesn't suffer from the problems that visual studio intellisense currently does when using clang as the toolchain. |
I would like to base it on GCC. |
@sumonero sure, that works as well. The specific code in question compiles fine with gcc as well, so I have no hard data to prefer clang over gcc in that respect. |
We actually do not use MSVC for IntelliSense (nor does Visual Studio). We use a different front-end which supports msvc, gcc, and clang modes. If you can open an new issue and show us which cases are providing incorrect squiggles for you, we can work to improve our existing implementation.
I'm not following you. Our goal is to provide an accurate linting experience, so if there are false errors reported, we'd like to know about them so we can fix our implementation. And is there a typo here? I thought the complaint was that there were incorrect squiggles in vscode. Using libclang as an additional engine is not out of the question, but it is also not a trivial change and not on the schedule at this point. I can pretty much promise you that when we do finish porting all the features they will not be implemented with libclang as the front-end. |
@bobbrow my apologies, I didn't think this was a forum where it would be appropriate to report issues with Visual Studio (and they really don't bother me that much as I use vscode predominantly). No, I have not had issues with vscode (I wasn't surprised as I didn't believe that it was doing sufficient analysis to detect the type of error that is being reported by intellisense on Visual Studio). In any case, I have the code that fails to compile with msvc here (reduced test case): https://godbolt.org/z/j5mJJ0 On intellisense (under Visual Studio not under vscode) the intellisense error is the same as the compiler error (which is what lead me to believed the front end is the same). As you can see the code compiles fine with gcc (trunk and 8.2) as well as clang (trunk and 6.0). I tested with msvc pre 2018 and msvc 2017 I was holding out libclang merely as a possible solution, anything that produces the same results is fine. |
@rtosman You're supposed to be able to get IntelliSense to work with clang in VS via setting the CppProperties.json correctly -- see https://docs.microsoft.com/en-us/cpp/ide/non-msbuild-projects?view=vs-2017#configure-intellisense-with-cpppropertiesjson . If you have problems getting that to work, I would recommend using the "Report a Problem..." feature in VS (I created an issue at https://developercommunity.visualstudio.com/content/problem/327884/c-compiler-fails-to-compile-lambda-that-captures-a.html ). The IntelliSense errors attempts to mimic the compiler used, so if it's set to msvc mode, it's more likely to give the errors that msvc (cl.exe) gives. |
@sean-mcmanus Correct me if I am wrong, but I believe the CppProperties.json file only works for "Open Folder" projects. Can CppProperties.json be used with solution file based projects as well? I am using a solution and I have set the toolchain to "llvm" in the project properties so I expect that setting to be passed on to intellisense. |
@rtosman Yeah, you're right, it's for Open Folder, not projects. If you're using the a project, the "mode" is supposed to be picked up by IntelliSense from the project. |
@sean-mcmanus Today I tried several times but got this error again after some trial. And progress bar at the upper side of window, Also it doesn't open any rename window this time. I believe it is something related to the parsing of files because I can see it is printing some lines within |
@karan-k-deepr If you can attach a debugger to the crashing process to get a call stack or provide more repro details. 0.26.0-insiders2 has a known crash when a document is opened related to the Outline view, but we haven't been able to repro any crash involving rename yet. |
I'm using 1.39.0 full info from about below and I get the dancing document status with C++ RENAME bar on the left-hand-side. There's content in there that's valid (Pending Rename items and Candidates for Rename items). The problem is, I have no idea what to click or press to get anything to apply. Eventually, if I click in the document and press a key, everything goes away, but the rename NEVER works. Was working fine up until I updated a day or two ago. :'( I'm on Ubuntu 16.04 LTS. Version: 1.39.0 |
@monkeycoder99, if you hover over the "PENDING RENAME" title menu bar, controls for the rename list will appear, see screenshot below. Use the checkmark button to commit or apply pending rename items. Editing a file will cause rename to cancel automatically. The rename view container will only appear if there are other references results or non-confirmed references like a string reference. If you do a rename on a symbol with results that are all confirmed references then rename will be applied automatically. |
@monkeycoder99 , I am not able to repro the issue of the buttons being invisible on dark theme with 0.26.0-insiders3. Is there a particular theme you're using that could be causing this? Can you verify if the buttons are also invisible for other dark themes? |
Can you check if the images for the buttons exist under the extension package path? The path to images should be under If the images exists, can you try modifying the path to the referenced images in file |
I have no assets directory in that location. Here's what's there:
|
We checked the shipped packages and they include the assets directory. Perhaps something went wrong when the extension was updated or installed? Can you try re-installing? |
Uninstall/reinstall fixed the problem. Not sure how this happened. Maybe something with an earlier install that switched to the Preview version? |
It's hard to guess how the folder was not installed. But if you get a consistent repo of a folder or files missing after installation, please create a new issue for it for us to look at. |
Rename is available in 0.26.0. |
I'm using with cmake tools and nothing happens except loading signal when renaming symbols. Extension info:
|
@ryancheung Can you a file a bug with more info? What type of "loading signal" do you see? Are other features working? |
I'm mean the light loading bar after doing a symbol rename. After a while, light loading bar disappears and no other action. |
@ryancheung What "light loading bar" are you referring to? In the "References: Results" pane or above the document or in the bottom right notification dialog? Can you provide a screenshot? Does the output window show any "Connection closed" or "Failed to read" messages? Does it only repro for certain workspaces? Does it only repro for 0.26.3-insiders2 or 0.26.2 also? |
Its above the document , no other message. I could provide gif later. |
When i have other extensions disabled, renaming works... |
I believe it's the cmake tools extension prevent the symbol renaming. I have cmake I think c++ extension is too busy and do not do symbol renaming job until the analysis job done. |
After disable compileCommands option in C++ entension configuration, symbol renaming works very well now. |
How did you disable it? I'm having the same issue, where I will get the light progress indicator near the top toolbar scrolling upon rename, and nothing happens... |
Having the same issue as described previously, Currently on 0.26.3-insiders. Looked through the logs, didnt see any weird output or disconnects, but I'm happy to collect logs and post if helpful! |
@bonfire62 That's the normal progress indicator as it's confirming references. Is that getting stuck? Are you using a multi-root workspace? |
Remove the json key from c++ extension configuration file. |
|
@bonfire62 What do you mean by "checking" ~650 files? Is it saying "searched" or "confirmed" in the progress UI? Searching is just lexical and should be fast and confirming is slow since it requires parsing. Both should scale to use up to your available cores, but you'll hit a performance issue if your machine doesn't have enough memory. The number of individual "uses" in files shouldn't noticeably affect performance. For tracking purposes, it would be better if you opened a new issue, although we do already have some other issues that include potential performance improvement discussions. |
It would be very useful if this tool also supported renaming of variables.
The text was updated successfully, but these errors were encountered: