[Lldb-commits] [PATCH] D26883: Demonstrate proposed new Args API

[Zachary Turner via lldb-commits]

One idea might be to have the entry contain 2 StringRefs. `str` and
`quoted_str`. This way you never get access to the underlying quote char,
just the full arg, either quoted or unquoted (although doing this would
still be better done orthogonally to this patch)
On Sat, Nov 19, 2016 at 5:48 AM Zachary Turner <zturner at google.com> wrote:

> Assuming we do that, what interface do you think would be simpler? We
> still need easy access to both a StringRef and a c_str(), since
> StringRef::data is not guaranteed to be null terminated, so the entry thing
> is still nice.
> On Sat, Nov 19, 2016 at 5:44 AM Zachary Turner <zturner at google.com> wrote:
>
> The quote char is only exposed as a means to not break existing code which
> depends on it (most of which, not surprisingly, is in the Args class itself.
>
> We could try to come up with a way to kill it, but that seems like a
> separate refactor (and perhaps quite difficult since different platforms
> have different rules)
> On Sat, Nov 19, 2016 at 5:23 AM Pavel Labath <labath at google.com> wrote:
>
> labath added a comment.
>
> I don't know how deep do you want this refactor to be, but there is one
> issue I would like us to consider, if only to decide it is out of scope of
> this change. I am talking about the `quote_char` thingy. The main problem
> for me is that I don't think it's possible to sanely define the meaning of
> that field. According to POSIX quoting rules (which our command line
> more-or-less follows) a single argument can be quoted in a great many ways,
> using various combinations of quote characters. For example, these are all
> valid ways to represent the argument `asdf` in a POSIX shell:
>
>   asdf
>   "asdf"
>   'asdf'
>   a"sd"f
>   "as"df
>   "as""df"
>   "as"'df'
>   "a"s'd'"f"
>   ... (you get my point)
>
> I don't think there is a self-consistent way to define what the
> `quote_char` field will be for each of these options. Moreover, I don't see
> why one would ever need to use that field. It can only encourage someone to
> try to "quote" the argument by doing `quote_char+value+quote_char`, which
> is absolutely wrong if you ever want that result to be machine parsable.(*)
> For proper quoting I think we should just have a free-standing `std::string
> quote_for_posix_shell(llvm::StringRef)` function (and maybe
> `quote_for_windows_cmd`, and whatever else quoting scheme we need), and
> then the user can decide which one to use based on who is going to be
> consuming it. Then we can just kill the `quote` field. The only thing is...
> I have no idea how much work that will be (but I am ready to chip in to
> make it happen).
>
> So, yea, if we decide not to do that, then I think the interface looks
> great. Otherwise, I think we can design a slightly simpler (and more
> consistent) one.
>
> (*) Bonus question: Try to start an executable under lldb, so that in
> enters `main()` with `argc=2` and `argv[1]="'"` I.e.,  as if it had been
> started this way via bash:
>
>   $ /bin/cat \'
>
>
> https://reviews.llvm.org/D26883
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161119/0b471330/attachment-0001.html>