1.22.4 and up break custom configuration providers #12832
Comments
|
I suspect this is likely related to #12632. Our extension was written back in 2018, but we've tried to keep the API somewhat up to date. |
|
Digging through more, I found what broke. The uri passed to me is now fully capitalized, whereas before it was not. And I'm doing a The issue is fixed by setting |
|
@ThadHouse I don't think setting "C_Cpp.caseSensitiveFileSupport" is the workaround we want users to use -- that is specifically for Windows folders that have case sensitivity enabled, and there might be other bugs introduced when that is used (#12732). @Colengms Was the caps change intentional? Should configuration providers be normalizing the casing of paths received from us when C_Cpp.caseSensitiveFileSupport is false? This seems like it might considered blocking for 1.22.9. |
sean-mcmanus
added
bug
Language Service
regression
sean-mcmanus
added
the
insiders
|
Normalizing the casing is problematic, because that has the potential to break on unix platforms where case sensitive file support is necessary. In fact we've ran into that exact issue before, because iirc back in the day we did toLower() on everything, but that broke some of our linux users. |
|
@ThadHouse The all caps casing change only occurs on Windows (with caseSensitiveFileSupport false). Our database and other internal paths are stored in uppercase in that scenario to facilitate case insensitive comparisons. |
|
Hi @ThadHouse . This should be fairly straight-forward for us to change to match the previous behavior. However, if the file system is case insensitive, it should be valid for URIs to contain case insensitive paths. VS Code will attempt to maintain stable casing for a URI, even going so far as to use incorrect/old casing after a file or folder has been renamed with new casing in the same session. ( microsoft/vscode#137256 ) Currently, we only request configurations for files that are open, but in the future we intend to request configurations for files in advance, to properly calculate the include graph. Since we don't have direct access to what VS Code may have previously referred to a file as, it will become necessary for custom configuration providers to properly handle case insensitive paths (when configured accordingly) that may not match the casing on disk and may not match the casing VS Code has settled on. While we can change this to make an effort (99%) to match the file system and VS Code casing, this will not be 100% reliable and should be addressed by the custom configuration provider handling case insensitivity when applicable. |
|
Is there any chance the extension API could provide the information on if the files are considered case insensitive or not? Right now theres a lot of checks that would have to be done to ensure the right type of comparison is used. Also, I saw above that you the database on case insensitive converts everything to upper case. What APIs are used to do that? And can those APIs be made accessible to the extension API? Languages other than English make case insensitive comparisons MUCH harder, especially since not all the APIs exist in node.js to make this easy. We've ran into multiple issues in the past where file checks have been broken by users running in other languages. |
|
Hi @ThadHouse . Currently, the C/C++ Extension will assume case insensitivity on Windows (unless
You could use
The native component of the C/C++ extension attempts to 'normalize' paths (remove |
Colengms
added
the
fixed
Environment
Bug Summary and Steps to Reproduce
Bug Summary:
Updating to 1.22.4 or newer breaks our existing C++ configuration provider.
Steps to reproduce:
./gradlew generateVsCodeto generate the intellisense configurationExpected behavior:
Includes are found and instellisense works.
Configuration and Logs
Configuration provider
"C_Cpp.default.configurationProvider": "vscode-wpilib",C/C++ Debug Logs
Diagnostic Logs
Other Extensions
WPILib VS Code Extension (https://github.com/wpilibsuite/vscode-wpilib), which provides the custom provider
Additional context
No response
The text was updated successfully, but these errors were encountered: