> On May 15, 2014, 4:32 p.m., Niklas Nielsen wrote:
> > Hey Chengwei - sorry for the tardy turnaround time on this review request.
> >
> > To me, it still seems like we are treating the symptoms of the real issue:
> > PATH is appended multiple times and the subsequent globbing adds the
> > available commands to same number of times.
> > The reason I am saying this is because the fix is difficult to understand
> > (it is not immediate that this is the problem it solves) and seems very
> > specialized for the "mesos help --help" and "mesos help help" case.
> >
> > Two things we could do:
> > 1) Don't add the new path unconditionally to the PATH variable i.e. check
> > if it is already there.
> > 2) In usage(), don't add duplicates to the commands from the globbed list
> > of candidates. This can be done pretty easy and local by using a set
> > instead of a list. Try to take a look at:
> > https://github.com/nqn/mesos/commit/01f77a1ab6d96f48765cc3539da1a1ebd4ba1d56
> >
> > Thoughts?
+1 Love the 'set'. Calling "mesos help foo" will still recurse into main and
dirname will still be prepended to PATH multiple times, but the commands will
not be printed multiple times.
"mesos help help" will give a weird error ("Not expecting --help before
command") instead of calling usage, but I think that's a pretty contrived case.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19180/#review43181
-----------------------------------------------------------
On May 15, 2014, 3:48 p.m., Chengwei Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19180/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 3:48 p.m.)
>
>
> Review request for mesos and Niklas Nielsen.
>
>
> Bugs: MESOS-1093
> https://issues.apache.org/jira/browse/MESOS-1093
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix mesos command parsing help
>
> Without this patch, "mesos help --help" will output below
>
> Not expecting '--help' before command
> Usage: mesos <command> [OPTIONS]
>
> Available commands:
> help
> resolve
> cat
> scp
> log
> tail
> execute
> ps
> local
> resolve
> cat
> scp
> log
> tail
> execute
> ps
> local
>
> Apparently all available commands printed twice, the "mesos help help"
> will print available commands 3 times.
>
> The root cause is the directory contains available mesos commands are
> added more than one times when recursive to main() again.
>
> Idea comes from Adam B.
>
> Review: https://reviews.apache.org/r/19180
>
>
> Diffs
> -----
>
> src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86
>
> Diff: https://reviews.apache.org/r/19180/diff/
>
>
> Testing
> -------
>
> done?
>
>
> Thanks,
>
> Chengwei Yang
>
>
