Open ThadHouse opened 1 day ago
ThadHouse
commented
1 day ago I suspect this is likely related to https://github.com/microsoft/vscode-cpptools/issues/12632. Our extension was written back in 2018, but we've tried to keep the API somewhat up to date.
ThadHouse
commented
1 day ago 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 String.startsWith from the URI passed in.
The issue is fixed by setting C_Cpp.caseSensitiveFileSupport in settings. There should be a way in the extension API to state that the provider requires case sensitive files, so this isn't an extra flag that needs to be set on every project.
sean-mcmanus
commented
1 day ago @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 (https://github.com/Microsoft/vscode-cpptools/issues/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.
ThadHouse
commented
1 day ago 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.
sean-mcmanus
commented
1 day ago @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.
Colengms
commented
22 hours ago 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. ( https://github.com/microsoft/vscode/issues/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.
ThadHouse
commented
22 hours ago 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.
Colengms
commented
22 hours ago Hi @ThadHouse . Currently, the C/C++ Extension will assume case insensitivity on Windows (unless C_Cpp.caseSensitiveFileSupport is set to enabled), and will assume case sensitivity on Linux and macOS. This is not ideal behavior, as Linux and macOS can also have case insensitive file systems, and all platforms can actually have a mixture of case sensitive and insensitive volumes/directories on the same system. So there's a possibility that the logic underlying this may improve/change at some point in the future.
provide the information on if the files are considered case insensitive or not
You could use os.platform to detect if running on Windows, then vscode.workspace.getConfiguration to get the value of C_Cpp.caseSensitiveFileSupport. Unfortunately, I don't know when/if we'll get to implement the 'ideal' behavior for detecting case sensitivity/insensitivity at any granularity. If that happens, we will likely need to provide updated guidance for config providers.
converts everything to upper case. What APIs are used to do that
The native component of the C/C++ extension attempts to 'normalize' paths (remove . and .. path parths). For case conversion on Windows, it looks like we're using native Win32 APIs: IsCharUpperW and CharUpperBuffW. But I think TypeScript should do the correct conversion simply using toUpperCase (and normalize).
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