labath added a comment.
It seems somewhat odd for a command to return error (one of the effects of that
for instance is to abort processing of batch scripts), but still perform some
changes. It might be more appropriate to call those warnings. However,
reporting warnings from here would require some plumbing, and the new behavior
is definitely more reasonable than the old one, so I don't think this patch
should be blocked on that.
================
Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:73
changed = true;
+ idx++;
} else {
----------------
does this actually change anything?
================
Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:113-114
} else {
error.SetErrorStringWithFormat(
"the replacement path doesn't exist: \"%s\"", replace_path);
}
----------------
If there are multiple non-existing paths, this will just return the last one
right? We should list all of them.
================
Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:146
changed = true;
+ idx++;
} else {
----------------
does this change anything?
================
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)
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77186/new/
https://reviews.llvm.org/D77186
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
