Handle multiple compile commands on client side (needs native server side support) #12960
Conversation
yiftahw
changed the title
| "uniqueItems": true, | ||
| "default": [] | ||
| } | ||
| ], | ||
| "markdownDescription": "Full path to `compile_commands.json` file for the workspace.", |
There was a problem hiding this comment.
This description should be updated
There was a problem hiding this comment.
updated the description, feel free to suggest something different, English is not my native language.
| @@ -684,7 +685,7 @@ export class CppProperties { | |||
| this.parsePropertiesFile(); // Clear out any modifications we may have made internally. | |||
| const config: Configuration | undefined = this.CurrentConfiguration; | |||
| if (config) { | |||
| config.compileCommands = path; | |||
| config.compileCommands = [path]; | |||
There was a problem hiding this comment.
Question for my team. Do we want to prefer storing the compileCommands property as a string in the actual c_cpp_properties.json file, or as an array? Either way is acceptable here because the json will be reparsed in handleConfigurationChange and converted back to an array.
One other thing to note is that we should change the Edit Configurations UI to make the input for compileCommands be a textarea instead, along with whatever associated changes should come with that. The answer to the question above would also guide the implementation for the storage of the data in the json file. (e.g. if we prefer string for the single entry case, we'd need to test for that when it comes via the UI and react accordingly)
| // Start tracking the fallback time for a new path. | ||
| this.compileCommandsFileWatcherFallbackTime.set(configuration.compileCommands, new Date()); | ||
| } | ||
| configuration.compileCommands = configuration.compileCommands?.map((path: string) => this.resolvePath(path)); |
There was a problem hiding this comment.
minor: I don't believe you need to use ?. here or on the line below because the existence of configuration.compileCommands was tested before entering this code block.
|
@bobbrow squiggles are now handled as well. I believe most of the work on the client side is done. please note there is a new telemetry entry and a new localized string: message = localize("multiple.paths.should.be.separate.entries", "Multiple paths should be separate entries in an array.");
newSquiggleMetrics.MultiplePathsShouldBeSeparated++; |
| } | ||
|
|
||
| currentConfiguration.compileCommands?.forEach((file: string) => { | ||
| paths.push(`${file}`) |
There was a problem hiding this comment.
a semicolon is missing on this line.
Thank you very much. Would you also be able to do the changes to the Edit Configurations UI in |
|
looking into it... putting the PR back to draft for now |
|
@bobbrow updated the UI panel. |
closes #7029
DONE:
c_cpp_properties.jsonschema andcompileCommandssetting to accept either a string or array of strings.settings.jsonschema anddefault.compileCommandssetting to accept either a string or array of strings.c_cpp_properties.jsonorsettings.jsonis placed inside an array of length 1.compile_commands.jsonis converted to a list of size 1 to consistently send an array (or undefined).c_cpp_properties.json.C/C++: Edit Configurations (UI)panel to handle multiplecompile_commands.jsonpaths.TODO: