clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed.
So looks like you didn't use "git commit --amend -a" again here. These changes are incremental changes on your patch which hasn't been submitted yet? ================ Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:21 + program_basename = "a.out.stripped" + program= self.getBuildArtifact(program_basename) self.build_and_launch(program) ---------------- add a space after "program" ================ Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:33-34 + program_module = active_modules[program_basename] + self.assertIn('name', program_module, 'make sure name is in module') + self.assertEqual(program_basename, program_module['name']) + self.assertTrue('symbolFilePath' not in program_module, 'Make sure a.out.stripped has no debug info') ---------------- Do a similar check for ='path' ``` self.assertIn('name', program_module, 'make sure "name" is in module') self.assertEqual(program_basename, program_module['name']) self.assertIn('path', program_module, 'make sure "path" is in module') self.assertEqual(program, program_module['path']) ``` In general, make sure you test every key that you have added support for. ================ Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:35 + self.assertEqual(program_basename, program_module['name']) + self.assertTrue('symbolFilePath' not in program_module, 'Make sure a.out.stripped has no debug info') + symbol_path = self.getBuildArtifact("a.out") ---------------- Check for the symbol status here: ``` self.assertEqual('Symbols not found.', program_module['symbolStatus']) ``` ================ Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:40 + program_module = active_modules[program_basename] + self.assertEqual(program_basename, program_module['name']) + self.assertEqual('Symbols loaded.', program_module['symbolStatus']) ---------------- verify 'path' again ================ Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:346 + object.try_emplace("symbolFilePath", symbol_path); } std::string loaded_addr = std::to_string( ---------------- Add an else clause here: ``` } else { object.try_emplace("symbolStatus", "Symbols not found."); } ``` ================ Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:352-357 + uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t)); + for (uint32_t i=0; i<num_versions; ++i) { + if (!version_str.empty()) + version_str += "."; + version_str += std::to_string(version_nums[i]); + } ---------------- fix indentation. ================ Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:358-360 +if (!version_str.empty()){ + object.try_emplace("version", version_str); +} ---------------- remove {} here for single line if statement Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82477/new/ https://reviews.llvm.org/D82477 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits