> On Feb 19, 2015, at 7:55 PM, Zachary Turner <[email protected]> wrote: > > Also, it seems to me there should be a function in Host called ShellExpand(). > On Mac or whichever other platform wanted to, it could call in to argdumper. > Other platforms could implement this some other way if they wanted to. This > way, after checking whether the flag is set, Platform would just need to call > Host::ShellExpand()
Agreed. We actually should: 1 - Change the eLaunchFlagGlobArguments to be eLaunchFlagShellExpansion 2 - Change all GlobArguments calls to be ExpandShellArguments 3 - Add a Host::ExpandShellArguments() that takes a ProcessLaunchInfo and returns a new one Another options is to keep eLaunchFlagGlobArguments and make that do what Zachary was talking about: internal glob expansion that would be consistent between all LLDB releases, and let eLaunchFlagShellExpansion perform shell expansion on each system differently. > > On Thu, Feb 19, 2015 at 5:57 PM Zachary Turner <[email protected]> wrote: > I'm ok with this as long as we don't actually rely on globbing in any tests > except for tests which are explicitly platform tests. It's currently > difficult to enforce this, because LaunchWithGlobbing is just an argument > that anyone can specify from anywhere to the ProcessLaunchInfo. And as a > result of this design choice, you end up with situations where certain > codepaths that aren't immediately clear to the external user ignore the > launch flags because the codepath didn't make its way through the Platform. > > Even the original test written for argdumper had this problem, where you > could specify the flag to launch with argdumper, and then because the code > decided to take a different path, it would ignore the argument and the test > would fail. So, if we can: > > a) write the original argdumper test against SBPlatform, and not against any > other public class > b) agree that argdumper should not be used in future tests unless it is a > test against SBPlatform > c) If at all possible, make this something other than a generic process > launch flag, since it is possible for LLDB to disrespect it without warning > or error if you go through the process plugin > d) If c is not possible, fix the case of launching a process through the > process plugin so that it will ask the platform if globbing is supported, and > if it says no, it will fail the launch. > > Then perhaps this is ok > > On Thu Feb 19 2015 at 5:55:16 PM Zachary Turner <[email protected]> wrote: > On Thu Feb 19 2015 at 5:43:55 PM Greg Clayton <[email protected]> wrote: > > > > That which will be part of testing the platform-specific bits of your > > platform > > > > There's inherent value in having as few platform specific bits as > > possible. When the same rules apply to every platform, > > a) everyone gets better test coverage > > b) everyone gets to benefit from everyone else's changes and bugfixes > > c) as a user, it makes it easier and more comfortable to switch platforms > > since you don't have to do a mental switch. > > d) It makes the code easier to maintain > > With the detriment that glowing might not behave like the shell does on your > system. It would be nice to also support other shells (tcsh, zsh, ksh, etc) > so that things would expand as the user expects them to. > We already have SBPlatformShellCommand for that right? _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
