Re: Review Request 19180: Fix mesos command parsing help

[Chengwei Yang <ch...@gmail.com>]

> On May 16, 2014, 7:32 a.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?
> 
> Adam B wrote:
>     +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.

Hi Niklas,

Sorry for late reply, so since the 2) improvement landed into usage(), so anyway we can't get duplicated commands in usage now though the 1) thing is still left to take. Do you like the first version of this patch? Which just do the small fix, add directory to PATH in the first through.


- Chengwei


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19180/#review43181
-----------------------------------------------------------


On May 16, 2014, 6:48 a.m., Chengwei Yang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19180/
> -----------------------------------------------------------
> 
> (Updated May 16, 2014, 6:48 a.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
> 
>