wallace requested changes to this revision.
wallace added inline comments.
This revision now requires changes to proceed.


================
Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py:26
     @skipIfWindows
-    @skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
+    #@skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
     def test_completions(self):
----------------
don't forget to remove the #


================
Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py:19-24
+    def verify_completions(self, actual_list, expected_list, 
not_expected_list=[]):
+        for expected_item in expected_list:
+            self.assertTrue(expected_item in actual_list)
+
+        for not_expected_item in not_expected_list:
+            self.assertFalse(not_expected_item in actual_list)
----------------
remove this


================
Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py:38-43
+        found = False
+        for line in lines:
+            if line.startswith('PATH='):
+                found = True
+                self.assertTrue(path_env_variable == line, "PATH environment 
variable not the same")
+        self.assertTrue(found, 'PATH environment variable not found')
----------------
you can write different tests:
- inheritEnvironment = false -> no PATH
- inheritEnvironment = true
 --  send an additional env in the config -> it should override the inherited
 -- PATH should be correct 


================
Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py:906
                                       stopCommands=options.stopCmds,
                                       exitCommands=options.exitCmds)
 
----------------
inheritEnv should be passed here


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1400-1412
   auto envs = GetStrings(arguments, "env");
-  if (!envs.empty())
+
+  // if arguments has launchWithDebuggerEnvironment
+  // then append current environment to g_vsc.launch_info
+  if (launchWithDebuggerEnvironment) {
+  // if (true) {
+    char** env_var_pointer = environ;
----------------
the env sent by the user in the launch.json config should have more priority 
than the inherited environment. For example, if env = ["X=A"] and the inherited 
env is ["X=Z", "Y=V"], then the resulting environment should be ["X=A", "Y=V"].
You should also write a test for that


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1405
+  if (launchWithDebuggerEnvironment) {
+  // if (true) {
+    char** env_var_pointer = environ;
----------------
remove this


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2250-2251
 void request_stepIn(const llvm::json::Object &request) {
+
+  return;
   llvm::json::Object response;
----------------
lol


================
Comment at: lldb/tools/lldb-vscode/package.json:89
+                                                               "type": 
"boolean",
+                                                               "description": 
"Inherit the VSCode Environment Variables",
+                                                               "default": false
----------------
Inherit the debugger environment when launching a process. Only works for 
binaries launched directly by LLDB.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74579/new/

https://reviews.llvm.org/D74579



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] ... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... walter erquinigo via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... walter erquinigo via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits
    • [Lldb-comm... Héctor Luis Díaz Aceves via Phabricator via lldb-commits

Reply via email to