----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19180/#review43181 -----------------------------------------------------------
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? - Niklas Nielsen 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 > >
