Grdowns/detect compile commands#2137
Grdowns/detect compile commands#2137bobbrow merged 21 commits intomastermicrosoft/vscode-cpptools:masterfrom grdowns/detect-compile-commandsmicrosoft/vscode-cpptools:grdowns/detect-compile-commandsCopy head branch name to clipboard
Conversation
…into grdowns/detect-compile-commands
|
|
||
| let showCompileCommandsSelection: PersistentState<boolean> = new PersistentState<boolean>("CPP.showPromptCompileCommands", true); | ||
| if (!showCompileCommandsSelection.Value) { | ||
| return; |
There was a problem hiding this comment.
[](start = 8, length = 3)
nit: looks like indentation is off by one space here
…Microsoft/vscode-cpptools into grdowns/detect-compile-commands
| return; | ||
| } | ||
|
|
||
| let message: string = "Detected compile_commands.json. Would you like to select a compile_commands.json for this configuration?"; |
There was a problem hiding this comment.
Do we want to separate the simple case (only 1 found) from the "we need more information" case so that the user only has to click once?
If there is only one compile_commands.json found, we could just say "Would you like to use [path] to configure IntelliSense?"
|
|
||
| let message: string = "Detected compile_commands.json. Would you like to select a compile_commands.json for this configuration?"; | ||
| let yes: string = "Yes"; | ||
| let later: string = "Later"; |
There was a problem hiding this comment.
"Later" feels like an affirmative choice. I think you meant "Ask Me Later". In the interest of keeping labels short, "Not Now" is another alternative.
There was a problem hiding this comment.
I did it to match VSCode's update message. I do agree that the alternatives you offer sound better.
There was a problem hiding this comment.
Thanks for the context. They are not asking a question, so "Later" is not an answer.
| let message: string = "Detected compile_commands.json. Would you like to select a compile_commands.json for this configuration?"; | ||
| let yes: string = "Yes"; | ||
| let later: string = "Later"; | ||
| let dontShowAgain: string = "Don't Show Again"; |
There was a problem hiding this comment.
I think "Don't Ask Again" matches the context of this popup a little better
| return; | ||
| } | ||
|
|
||
| let showCompileCommandsSelection: PersistentState<boolean> = new PersistentState<boolean>("CPP.showPromptCompileCommands", true); |
There was a problem hiding this comment.
PersistentState is global. We might only want to disable it for the current workspace. You can use PersistentFolderState for that.
bobbrow
left a comment
There was a problem hiding this comment.
We need to settle the UI presentation
…Microsoft/vscode-cpptools into grdowns/detect-compile-commands
| return; | ||
| } | ||
|
|
||
| let message: string = "Would you like to use compile_command.json files to auto-configure IntelliSense?"; |
There was a problem hiding this comment.
nit: these let should be const
| } | ||
|
|
||
| let message: string = "Detected compile_commands.json. Would you like to select a compile_commands.json for this configuration?"; | ||
| let message: string = "Would you like to use compile_command.json files to auto-configure IntelliSense?"; |
| for (let i: number = 0; i < paths.length; i++) { | ||
| items.push({label: paths[i], description: "", index: i}); | ||
| } | ||
| items.push({ label: "Edit Configurations...", description: "", index: paths.length }); |
There was a problem hiding this comment.
I didn't see this in the spec. You should ask Rong if she wants this or not. My vote is that it's unnecessary.
| items.push({ label: "Edit Configurations...", description: "", index: paths.length }); | ||
|
|
||
| return vscode.window.showQuickPick(items, options) | ||
| .then(selection => { |
There was a problem hiding this comment.
nit: please indent the then block to match coding style elsewhere in the file.
| if (index < 0) { | ||
| return; | ||
| } | ||
| if (index >= params.paths.length) { |
There was a problem hiding this comment.
if we keep this, please add a comment that this is because "edit configurations" is added to the list.
There was a problem hiding this comment.
Removed after speaking with Rong 😄
| } | ||
|
|
||
| let compileCommandStr: string = params.paths.length > 1 ? "a compile_commands.json file" : params.paths[0]; | ||
| let folderStr: string = vscode.workspace.workspaceFolders.length > 1 ? "the " + this.Name : "this"; |
There was a problem hiding this comment.
vscode.workspace.workspaceFolders [](start = 32, length = 33)
should always check that vscode.workspace.workspaceFolders is defined before asking for .length
Respond to a
compileCommandsPathsmessage by prompting the user to select acompile_commands.jsonwhich will then populatecompileCommandsinc_cpp_properties.jsonfor the active configuration.