> On Apr 6, 2018, at 2:07 AM, Pavel Labath via Phabricator
> <revi...@reviews.llvm.org> wrote:
>
> labath added inline comments.
>
>
> ================
> Comment at: packages/Python/lldbsuite/test/settings/TestSettings.py:544-545
> + # the actual name and via .experimental.
> + cmdinterp.HandleCommand("settings set target.arg0 first-value",
> result)
> + self.assertEqual(result.Succeeded(), True)
> + cmdinterp.HandleCommand("settings show target.arg0", result)
> ----------------
> Isn't this basically what `self.expect` would do (only with better logging
> and error messages)?
Ah, I didn't see that self.expect would allow me to specify whether to expect
an error return or not. Yes I can write this in terms of self.expect more
cleanly.
BTW what does the documentation for self.expect in lldbtest.py mean when it
refers to "golden input"? It uses the phrase a few times and I can't figure
out what it's talking about. Maybe that term was in the documentation from
long ago and not a recent addition.
>
>
> ================
> Comment at: source/Interpreter/OptionValueProperties.cpp:210-215
> + llvm::SmallVector<llvm::StringRef, 8> components;
> + name.split(components, '.');
> + bool name_contains_experimental = false;
> + for (const auto &part : components)
> + if (Properties::IsSettingExperimental(part))
> + name_contains_experimental = true;
> ----------------
> Not a big deal, but I would expect that the magicness of `experimental` would
> kick in only for the components which are come after `experimental` keyword.
> So something like `target.experimental.non-existant-setting` should be
> subject to the magic behavior, but
> `target.non-existant-setting.experimental.foo` should not (?)
Yeah, I was debating whether to do it properly like that or not. The
GetSubValue() method is recursive and would need a new bool &is_experimental
argument, but it would need to be initialized to false before GetSubValue was
called, or GetSubValue would need to be renamed to GetSubValueImpl and a
GetSubValue function that initializes it to false and then calls
GetSubValueImpl would need to be done. It's not terrible, but I couldn't make
up my mind if it was worth the trouble to correctly error out on
target.non-existant-setting.experimental.foo or not.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
