jingham added a comment.
To Raphael's points:
Argument completion handlers are set by the command object implementing
HandleArgumentCompletion and dispatching to the
CommandCompletions::eDiskFileCompletion. An example of this is in
CommandObjectProcessLaunch. Note, arguments currently have a kind (as do
options) but for historical reasons, those kinds don't automatically bind to
completers. Be nice to finish that bit of the design at some point... But
till then, this way though boilerplate is straightforward.
The StreamFile constructor doesn't handle ~. Maybe it should? Or you could
make a FileSpec, they do handle ~ completion, and there's a StreamFile
constructor that takes a FileSpec.
Breakpoints use "breakpoint write" and "breakpoint read" to save themselves.
Maybe it would be clearer if we use the same word for saving breakpoints and
saving settings?
I also wonder if it would be good to add a "settings {import, read?}" command
to go along with the settings {export,write}. It is a little odd to do
"settings export" to export the settings, and "command source" to read them
back in. That's relying on the accident that you are exporting the settings as
commands, which (a) would be better not to tie ourselves to and (b) users
shouldn't have to know that...
It also might be nice to have the output file be a -f option and then I could
do:
(lldb) settings export -f foo.cmds
to export all settings and:
(lldb) settings export -f foo.cmds target
to export all the target commands, etc... That might be more flexible for
ordinary use. If you do that, the easiest way to specify a completer is by
using OptionGroupFile.
================
Comment at: source/Commands/CommandObjectSettings.cpp:213
+ }
+
// Split the raw command into var_name and value pair.
----------------
"settings clear" also clears the settings value. I'm not sure I'm in favor of
having two ways of doing this, especially when the second one is undocumented.
Presumably this isn't intrinsic to exporting settings, so it should be done as
a separate patch anyway.
================
Comment at: source/Commands/CommandObjectSettings.cpp:331
+ var_name_arg.arg_repetition = eArgRepeatOptional;
+
+ // There is only one variant this argument could be; put it into the
----------------
Shouldn't this be eArgRepeatPlain? You don't do anything in the case where
there's no filename argument.
BTW, I don't think these argument repeat counts are actually enforced. They
are used to write out the help strings, but not to check incoming arguments for
commands so far as I can see. The development of these and the argument kind
specifications stalled when the person who originally worked on them left, and
is waiting for somebody to pick them back up again.
================
Comment at: source/Commands/CommandObjectSettings.cpp:356
+ // Open file for dumping the exported settings.
+ const std::string export_path = args.GetArgumentAtIndex(0);
+ const uint32_t options = File::eOpenOptionWrite |
----------------
You don't know that you actually got an argument. You should check that here.
https://reviews.llvm.org/D52651
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
