<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 25, 2015, at 10:29 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="">zturner@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Well, I agree in principle that the API is meant to support process launching.  But if its utility can be trivially extended to support other use cases, then why not?<br class=""><div class=""><br class=""></div></div></div></blockquote><div><br class=""></div><div>YAGNI - also, ProcessLaunchInfo isn’t that hard to construct</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">FWIW, when I decided to do this, it was only to enable the functionality for other platforms (i.e. moving from specific dirs to common), because only Windows is going to have a different codepath here so it didn't make sense to increase the code debt for everyone just because of Windows.  Then changing of the signature was just something I noticed on the side.  I can change it back if people feel strongly, </div></div></div></blockquote><div><br class=""></div><div>Please do</div><div>FWIW, I think that the environment variables in the launch info should ALSO be passed down (they aren’t now, but it’s not by design), so with your change, I’d have to go add yet another argument when I have time to get that piece of work done - and by then, I am passing essentially all the guts of a ProcessLaunchInfo, but not the actual ProcessLaunchInfo for no clear reason</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">but at the same time, I really like making code as general as possible, as long as the generality doesn't hinder the readability or usability for the primary use case (which in this case I don't think it does)</div></div></div></blockquote><div><br class=""></div><div>I don’t think I agree with that (i.e., I think it does make our real use case for this less readable and usable)</div><br class=""><blockquote type="cite" class=""><div class=""><br class=""><div class="gmail_quote">On Wed, Feb 25, 2015 at 10:24 AM Enrico Granata <<a href="mailto:egranata@apple.com" class="">egranata@apple.com</a>> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">-    </div><div class="">-    void</div><div class="">-    SetShellExpandArguments (bool glob);</div><div class="">-    </div><div class="">+</div><div class="">+    void SetShellExpandArguments(bool expand);</div><div class="">+</div><div class=""><br class=""></div><div class="">Yes, totally</div></div><div style="word-wrap:break-word" class=""><div class=""><br class=""></div><div class=""><div class="">-    static Error</div><div class="">-    ShellExpandArguments (ProcessLaunchInfo &launch_info);</div><div class="">-    </div><div class="">+    static Error ShellExpandArguments(llvm::StringRef input, llvm::StringRef working_dir, std::vector<std::string> &expanded);</div><div class="">+</div></div><div class=""><br class=""></div></div><div style="word-wrap:break-word" class=""><div class="">Why? I see no benefit to doing this. This API is clearly meant to support process launching. This change seems a net loss to me.</div><div class=""><br class=""></div><div class="">The actual moving of the argdumper logic to Host/common is fine (as long as the mechanism for individual platforms to opt out is trivial, that is) - the change in signature from ProcessLaunchInfo to bunch-of-stuff not so much</div><div class=""><br class=""></div><div class=""><div class="">-    if (launch_info.GetFlags().Test(eLaunchFlagShellExpandArguments))</div></div><div class=""><br class=""></div><div class="">This should be fine do to do, yes</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""></blockquote></div></div><div style="word-wrap:break-word" class=""><div class=""><blockquote type="cite" class=""><div class="">On Feb 24, 2015, at 7:31 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank" class="">zturner@google.com</a>> wrote:</div><br class=""></blockquote></div></div><div style="word-wrap:break-word" class=""><div class=""><blockquote type="cite" class=""><div class="">Anyone have thoughts on this?  If there's no suggestions I'd like to commit, but I'll give another day or two for comments.<br class=""><br class=""><br class=""><a href="http://reviews.llvm.org/D7805" target="_blank" class="">http://reviews.llvm.org/D7805</a><br class=""><br class="">EMAIL PREFERENCES<br class="">  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank" class="">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br class=""><br class=""><br class=""><br class=""></div></blockquote></div></div><div style="word-wrap:break-word" class=""><div class=""><blockquote type="cite" class=""><div class="">_______________________________________________<br class="">lldb-commits mailing list<br class=""><a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank" class="">lldb-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br class=""></div></blockquote></div><br class=""><div class="">
<div style="letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; word-wrap: break-word;" class=""><div style="letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; word-wrap: break-word;" class=""><div style="letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; word-wrap: break-word;" class=""><div style="letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; word-wrap: break-word;" class=""><div style="letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; word-wrap: break-word;" class=""><div class="">Thanks,</div><div class=""><i class="">- Enrico</i><br class="">📩 egranata@<font color="#ff2600" class=""></font>.com ☎️ 27683</div><div class=""><br class=""></div></div></div></div></div></div><br class=""><br class="">
</div>
<br class=""></div></blockquote></div>
</div></blockquote></div><br class=""><div class="">
<div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Thanks,</div><div class=""><i class="">- Enrico</i><br class="">📩 egranata@<font color="#ff2600" class=""></font>.com ☎️ 27683</div><div class=""><br class=""></div></div></div></div></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<br class=""></body></html>