monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Monotone-devel] [patch]--depth for monotone ls


From: Joel Reed
Subject: Re: [Monotone-devel] [patch]--depth for monotone ls
Date: Wed, 18 May 2005 09:05:19 -0400
User-agent: Mutt/1.5.5.1i

On Tue, May 17, 2005 at 11:09:24PM -0600, Derek Scherger wrote:
> Joel Reed wrote:
> > (%:~/tmp/root) monotone --db=../mt.db --depth=1 ls known .
> > file
> > 
> > (%:~/tmp/root) monotone --db=../mt.db --depth=2 ls known . 
> > a/file
> > file
> > 
> > (%:~/tmp/root) monotone --db=../mt.db --depth=3 ls known . 
> > a/b/file
> > a/file
> > file
> 
> I'm not sure if ls known is considered a working copy command but you
> probably don't need the --db setting after the first time.
> 
> > This gives me what I need. When no --depth is provided "." works as
> > before. Giving the whole subtree from that point.
> > 
> > A few questions:
> > 
> > 1) functionality look ok?
> 
> looks ok to me, and yeah, it's certainly a nice small patch, good work!
> 
> > 2) --depth as param name ok?
> 
> not sure. I wonder if simply --local or --nonrecursive would be better
> so that --depth can always mean ancestry depth. no major objection to
> --depth though. also, I wonder whether --depth=0 means current dir and
> --depth=1 means one level deeper? that would have been what I expect,
> but I don't know what find's --depth semantics are off the top of my head.

find use's --maxdepth for this, so i'll use that option name, and match
its numbering scheme.

> > 3) patch look ok (-testcase & Changelog!)
> 
> personally I'd prefer 'if (depth != -1)' over 'if (-1 != depth)'.

ok, i was thinking of making the code more readable perhaps with 
a line 

  bool user_supplied_mindepth = (depth != -1);

> > 4) I want to add this to "list" obviously, but any other subcommands
> >      you'd nominate for working with this option? (must be something
> >      that uses restriction code!)
> 
> my vote would be for all restrictable commands to take the option. so
> status, diff, commit, revert, ls unknown/ignored/missing and any others.
> it appears that all you need to do is enable the option for them so it
> seems easy enough to add.

ok, i'll add the option to all of those commands. thanks for the feedback!

jr





reply via email to

[Prev in Thread] Current Thread [Next in Thread]