Add wildcard support for includeDir

[yne]

For example, using the following tree:

• rootUri/
  • sources/
    • main/
      • main.c
    • vendor/
      • khash.h
  • headers/
    • C001
      • a.h
    • C002
      • b.h

The following directory	will give
sources/**	plain old recursive path like before
headers/C*/	dynamic path
missing/	reported as "not found"
missing/*	expanded as empty result (silent fail)

Fixes #723

[yne]

@microsoft-github-policy-service agree

[yne]

npm run unitTests succeed (Exit code: 0) ✔️

But it also output the following message:

Unable to resolve your shell environment: Unexpected exit code from spawned shell

[sean-mcmanus]

Thanks for submitting this. We've been busy reviewing other PRs this past week but we'll try to get around to review this next week.

[sean-mcmanus]

Does this expand subdirectories on the right side, such that it could potentially generate a large number of directories? If so, can that behavior be suppressed in favor of our existing recursive includes support? We want to avoid that because it negatively effects performance of our parser. It's okay for "internal" wildcard directories, since those are not as likely to generate a large number of directories.

[yne]

The right /** expression is kept out of the new glob process.
So only the old one can see it and can process it.

However, if there are any middle ** expression, they will be expanded recursively by this new expansion process :/

I understand your worries about the performance impact. luckily for us fast-glob provide a globstar option to disable this ** behaviour.

I've amended my commit to specify a globstar: false option.

[sean-mcmanus]

FYI, we've been unexpectedly busy and haven't had time to review this yet. Our current target would be 1.15.0, since 1.14.x is entering stabilization mode.

[sean-mcmanus]

We've been busy with the 1.14.x release. I may be able to review this next week or the week after.

resolved

[sean-mcmanus]

I don't see this change actually working. First, the changes only appear to be in getErrorsForConfigUI which is only invoked when the configuration UI is loaded. But even when I do that, I get 0 results for

            "includePath": [
                "${workspaceFolder}/*/inc"
            ],

How are you testing your change?

Also, there might be an issue with an incorrect squiggle appearing in the json UI editor for c_cpp_properties.json.

[yne]

I indeed tested through the Configuration UI.
I'll try from other means (direct JSON configuration edit).

What kind of squiggle ?

[sean-mcmanus]

I indeed tested through the Configuration UI. I'll try from other means (direct JSON configuration edit).

What kind of squiggle ?

I see "Cannot find "<workspace folder>\\inc"

[yne]

that's interesting.
I didn't tested on windows so I didn't got the backslashed star case (\*).
maybe it's preventing the star expansion.

I'll try to find a windows machine to test.

thanks for the info. I'll keep you informed.

[gerlero]

@yne As someone subscribed to #723, any update on this?

[yne]

Hi @gerlero, the only windows machine I found was my company machine which has network restrictions (no github push/pull allowed)

I could circumvent they restrictions but that's risky.

[yne]

@gerlero : do you have a windows machine ?

[sean-mcmanus]

If you have a theoretical fix we could test it for you. Or can you download a zip of repo-branch for use on the Windows machine and then just re-do the fix/push on your other machine?

[yne]

Current status of my corporate windows machine and it "Secure Web Gateway" (aka man in the middle HTTPS proxy) :

• git clone failed (tried ssh and https) => Connection reset (probably blocked by firewall)
• so downloading the zip + extract in my home
• installing node 18.16.0 LTS from msi (required me administrator privilege; which not much people have).
• yarn installing via corepack prepare yarn@stable --activate did not work even with setting additional certificates
• So I manually put yarn.js 3.5.1 in my /appdata/ + corepack prepare yarn@3.5.1 --activate
• From the Extension folder, running yarn install fail with the .log given bellow
• pressing F5 while being in the Extension folder give me some kind of "yarn: no such command" that only seems to occur in PowerShell based terminal (cmd works)

# This file contains the result of Yarn building a package (cpptools@workspace:.)
# Script name: postinstall


> cpptools@1.14.3-main download-api
> vscode-dts dev

Downloading vscode.proposed. [32mterminalDataWriteEvent [0m.d.ts
To:   C:\Users\yne\vscode-cpptools-release\Extension\vscode.proposed.terminalDataWriteEvent.d.ts
From: [https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.terminalDataWriteEvent.d.ts](https://www.google.com/url?q=https%3A%2F%2Fraw.githubusercontent.com%2Fmicrosoft%2Fvscode%2Fmain%2Fsrc%2Fvscode-dts%2Fvscode.proposed.terminalDataWriteEvent.d.ts&sa=D&sntz=1&usg=AOvVaw33lAJq5oGA3qzOxELyOTiz)
Read more about proposed API at: [https://code.visualstudio.com/api/advanced-topics/using-proposed-api](https://www.google.com/url?q=https%3A%2F%2Fcode.visualstudio.com%2Fapi%2Fadvanced-topics%2Fusing-proposed-api&sa=D&sntz=1&usg=AOvVaw3WjbXUCKa3m6RcBPUfe9Pp)
node:events:491
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TLSWrap.onStreamRead (node:internal/stream_base_commons:217:20)
Emitted 'error' event on ClientRequest instance at:
    at TLSSocket.socketErrorListener (node:_http_client:502:9)
    at TLSSocket.emit (node:events:513:28)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -4077,
  code: 'ECONNRESET',
  syscall: 'read'
}

Node.js v18.16.0

Having battled with JavaScript ecosystem in the past, I'm not surprised having so much hard time doing a simple environment setup. However I was wondering if some of those issues are more windows or Node related ?
Am I close to have a working enviroment or shall I just give up and download a VM ?

[sean-mcmanus]

We're still using yarn 1.22.x and it may not work with yarn 3.5.

[yne]

It get stuck at the same step even with yarn 1.22

I tried running a W11 VM but it was unbearable on my 8Go laptop

So I finally got my hands on a dualboot PC and it seems to be working, and without a surprise the issue was that the windows separator \ are considered as escape character. This makes fast-glob consider the path as dynamic (https://www.npmjs.com/package/fast-glob#what-is-a-static-or-dynamic-pattern), But it's fixed now:

I'll just need to get the changes back into git.

[yne]

@sean-mcmanus I've rebased onto main and committed the fix in a separate commit for easier review.
It shall now work, but note that using glob with a slash-ending path may return an empty match

.

[sean-mcmanus]

@yne You're hitting some linter issues...they look minor/easy to fix. We have a gulp lint npm script you can run locally.

[yne]

• lint fixed
• rebased onto origin/main
• testing undone because whatever branch I launch on, I get a braindead VSCode C++ Extension (no Ctrl+click available, no yellow underlining when I give a bad C_Cpp.default.includePath , no settings display when I click on the add debug configuration cog...) yet I get no error on the host "Debug Console" (beside a Buffer() is deprecated), and no F12 error on the created VSCode window.

I'll let you handle this part 💪

[sean-mcmanus]

@yne Yeah, it's possible "main" requires changes in the binary that only available with 1.16.1 which will be released later today.

[sean-mcmanus]

@yne FYI, main is "locked down" for the 1.16.2 release (maybe for Thursday) until we merge to the release branch. I think this can get into 1.17.0 (pre-release).

[fearthecowboy]

I'm OK with this - I haven't tested the functionality.

This can get merged ahead of #11085

[browntarik]

After attempting to test the feature myself, the support for dynamic paths does not seem to work, as a result we will delay checking this in. The example configuration did not yield the correct include paths.
Configuration
{
"configurations": [
{
"name": "Win32",
"includePath": [
"sources/nest*/**"
],
"defines": [
"_DEBUG",
"UNICODE",
"_UNICODE"
],
"windowsSdkVersion": "10.0.22000.0",
"cStandard": "c17",
"cppStandard": "c++17",
"intelliSenseMode": "windows-msvc-x64"
}
],
"version": 4
}

Log Output:
-------- Diagnostics - 6/26/2023, 3:23:19 PM
Version: 1.16.1-main
Current Configuration:
{
"name": "Win32",
"includePath": [],
"defines": [
"_DEBUG",
"UNICODE",
"_UNICODE"
],
"windowsSdkVersion": "10.0.22000.0",
"cStandard": "c17",
"cppStandard": "c++17",
"intelliSenseMode": "windows-msvc-x64",
"compilerPathIsExplicit": true,
"cStandardIsExplicit": true,
"cppStandardIsExplicit": true,
"intelliSenseModeIsExplicit": true,
"mergeConfigurations": false,
"compilerPath": "s",
"browse": {
"path": [
"${workspaceFolder}"
],
"limitSymbolsToIncludedHeaders": true
}
}
cpptools version (native): 1.16.1.0
Translation Unit Mappings:
[ C:\Users\browntarik\Desktop\Test\sources\nested\nested.h ]:
C:\Users\browntarik\Desktop\Test\sources\nested\nested.h
Translation Unit Configurations:
[ C:\Users\browntarik\Desktop\Test\sources\nested\nested.h ]:
Process ID: 29076
Memory Usage: 51 MB
Compiler Path: C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe
Includes:
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\include
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\um
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\ucrt
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\shared
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\winrt
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\cppwinrt
Defines:
_DEBUG
UNICODE
_UNICODE
Standard Version: ms_c++17
IntelliSense Mode: windows-msvc-x64
Other Flags:
--header_only_fallback
Total Memory Usage: 51 MB

------- Workspace parsing diagnostics -------
Number of files discovered (not excluded): 5028

[sean-mcmanus]

@yne Can headers/C*/** be made to work? The expanded paths should just preserve the "**" at the end so we can process those. Or did you want to exclude that for some reason?

[yne]

the ending ** are kept out of glob process and re-added post-globbing

It worked on windows in my last fix but I cannot test it again now (see: my last braindead vscode issue)

Is there any formal verification procedure we could agreed upon ?

I'll let you tell me when I can start debugging again (after the problematic plugin migration?)

[browntarik]

The feature does work as long as "**" is excluded. I will make a note for that to be a future improvement and we can go ahead and check this in for now.

[sean-mcmanus]

@browntarik @yne On Windows (and Linux/Mac), headers/C*/ doesn't work, but headers/C* works.

[sean-mcmanus]

@yne FYI, you don't need to fix this. I think @browntarik was going to (i.e. #11135 is assigned to him). The shipped 1.16.3 binaries should be usable for testing.

[haolly]

@sean-mcmanus I need this feature, how can I try it ASAP ?

[sean-mcmanus]

@haolly It should be available in our 1.17.0 release. We're not sure when that will be yet, because we're still working on fixing some issues.