labath added inline comments.
================
Comment at: include/lldb/Utility/Args.h:121
+ bool GetFlattenWindowsCommandString(std::string &command) const;
+
----------------
Hui wrote:
> labath wrote:
> > I am sorry for being such a pain, but I don't think this is grammatically
> > correct, as `get` and `flatten` are both verbs. I'd call this either
> > GetFlatten**ed**WindowsCommandLine or just plain FlattenWindowsCommandLine.
> There are two kinds of sources of args in this discussion:
>
> - from lldb console through stdin which is not raw.
> - from LLDB_SERVER_LOG_FILE environment variables which are raw
>
> We expect to call a GetFlattenedWindowsCommandString for raw input. However
> it seems not the case for now.
>
> What about adding a TODO in the following in ProcessWindowsLauncher.
>
> Would like a solution to proceed.
>
> +
> +bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
> + if (args.empty())
> + return false;
> +
> + std::vector<llvm::StringRef> args_ref;
> + for (auto &entry : args.entries())
> + args_ref.push_back(entry.ref);
> +
> + // TODO: only flatten raw input.
> + // Inputs from lldb console through the stdin are not raw, for example,
> + // A command line like "dir c:\" is attained as "dir c":\\". Trying to
> flatten such
> + // input will result in unexpected errors. In this case, the flattend
> string will be
> + // interpreted as "dir c:\\ which is a wrong usage of `dir` command.
> +
> + command = llvm::sys::flattenWindowsCommandLine(args_ref);
> + return true;
> }
> +} // namespace
>
I am afraid you lost me here. By the time something gets to this function, it
has already been parsed into individual argv strings. I am not sure which ones
do you consider raw, or why should it make a difference. (lldb builtin
interpreter has a concept of "raw" commands, but I don't think that's what you
mean here)
This function should take those argv strings, no matter what they are, and
recompose them into a single string. If those strings are wrong, then it's
output will also be wrong, but that's not a problem of this function -- it's a
problem of whoever parsed those strings.
I won't have access to a windows machine this week to check this out, but I can
take a look at that next week. In the mean time, I would be fine with just
xfailing the single test which fails because of this. I think that's a good
tradeoff, as this would fix a bunch of other tests as well as unblock you on
your lldb-server work.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56230/new/
https://reviews.llvm.org/D56230
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
