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?
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, 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) On Wed, Feb 25, 2015 at 10:24 AM Enrico Granata <egran...@apple.com> wrote: > - > - void > - SetShellExpandArguments (bool glob); > - > + > + void SetShellExpandArguments(bool expand); > + > > Yes, totally > > - static Error > - ShellExpandArguments (ProcessLaunchInfo &launch_info); > - > + static Error ShellExpandArguments(llvm::StringRef input, > llvm::StringRef working_dir, std::vector<std::string> &expanded); > + > > 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. > > 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 > > - if (launch_info.GetFlags().Test(eLaunchFlagShellExpandArguments)) > > This should be fine do to do, yes > > On Feb 24, 2015, at 7:31 PM, Zachary Turner <ztur...@google.com> wrote: > > 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. > > > http://reviews.llvm.org/D7805 > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ > > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > Thanks, > *- Enrico* > 📩 egranata@.com ☎️ 27683 > > > > >
_______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
