[Lldb-commits] [PATCH] D77186: [source maps] Ensure all valid source maps are added instead of failing with the first invalid one

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 4 inline comments as done.
wallace added inline comments.


================
Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:73
             changed = true;
+            idx++;
           } else {
----------------
labath wrote:
> does this actually change anything?
Indeed. It's used in line 70 as the index to replace in the replace operation


================
Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:113-114
         } else {
           error.SetErrorStringWithFormat(
               "the replacement path doesn't exist: \"%s\"", replace_path);
         }
----------------
labath wrote:
> If there are multiple non-existing paths, this will just return the last one right? We should list all of them.
good catch


================
Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:146
             changed = true;
+            idx++;
           } else {
----------------
labath wrote:
> does this change anything?
Same as above, it's used in line 144


================
Comment at: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py:45
+        src_map_cmd = 'settings set target.source-map . "%s" . "%s"' % (src_dir + "invalid_path", src_dir)
         self.dbg.HandleCommand(src_map_cmd)
+        assertSourceMaps(src_path)
----------------
labath wrote:
> It would be good to follow this up with a `settings show`, both to test/document the expected final value of the setting, and also to catch any issues early.
good one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77186