You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2008/01/26 20:38:06 UTC

Re: UI interpretation quetsions: --changelist + --depth

On Jan 26, 2008 12:46 PM, Julian Foad <ju...@btopenworld.com> wrote:
> Hi Mike and everyone!
>
> C. Michael Pilato wrote:
> > Karl Fogel wrote:
> >
> >> Ummm, how about:
> >>
> >>   For most commands, '--changelist FOO' restricts the scope to items
> >>   in the intersection of changelist FOO and the specified (or default)
> >>   depth.  But for *some* commands, [...]
> >>   '--changelist FOO' changes their non-infinity default depth to
> >>   infinity, so that the changelist becomes the only restriction when
> >>   no depth restriction is explicitly given.
> >>
> >> [...]
> >>
> >> I know people will cry "inconsistency", but I'm not sure consistency
> >> is a major factor in good UI anyway.  Meeting intuitive expectations
> >> is more important.
> >>
> >> -Karl
> >
> > [...] perhaps we should just roll with the
> > easy-to-explain "changelist is always a filter, period" explanation.  After
> > all, it really isn't so hard to change 'svn info --changelist foo' into
> > 'svn info -R --changelist foo'.
>
> Er, you what?! If the user issues a command with '--changelist foo' as the only
> file selection criterion, then he/she wants to act on the files in that
> changelist. I can't see any other possible interpretation for 'svn info
> --changelist foo'. The same goes for all subcommands - I've just run through
> them all in my head.
>
>  From a user's point of view, I interpret '--changelist foo' as "act on the
> targets specified in changelist 'foo'", just the same as '--targets foo' means
> "act on the targets specified in file 'foo'". Any default recursion or
> non-recursion that a particular command assumes would come after that step, in
> my head.

FWIW, it sounds like you both are saying it should work the same way.
The part that I suspect Julian was missing was that before you made
this change many of the subcommands  were essentially adding what was
in the changelist to what was in CWD which really does not make any
sense.  And clearly I think a user expects:

svn info --changelist foo /path

To show the info for the items in /path that belong to changelist foo.
 That is what Julian says it should do and that is what Mike just made
it do with these changes.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: UI interpretation quetsions: --changelist + --depth

Posted by Mark Phippard <ma...@gmail.com>.
On Jan 26, 2008 3:51 PM, David Glasser <gl...@davidglasser.net> wrote:
>
> On Jan 26, 2008 12:38 PM, Mark Phippard <ma...@gmail.com> wrote:
> >
> > On Jan 26, 2008 12:46 PM, Julian Foad <ju...@btopenworld.com> wrote:
> > > Hi Mike and everyone!
> > >
> > > C. Michael Pilato wrote:
> > > > Karl Fogel wrote:
> > > >
> > > >> Ummm, how about:
> > > >>
> > > >>   For most commands, '--changelist FOO' restricts the scope to items
> > > >>   in the intersection of changelist FOO and the specified (or default)
> > > >>   depth.  But for *some* commands, [...]
> > > >>   '--changelist FOO' changes their non-infinity default depth to
> > > >>   infinity, so that the changelist becomes the only restriction when
> > > >>   no depth restriction is explicitly given.
> > > >>
> > > >> [...]
> > > >>
> > > >> I know people will cry "inconsistency", but I'm not sure consistency
> > > >> is a major factor in good UI anyway.  Meeting intuitive expectations
> > > >> is more important.
> > > >>
> > > >> -Karl
> > > >
> > > > [...] perhaps we should just roll with the
> > > > easy-to-explain "changelist is always a filter, period" explanation.  After
> > > > all, it really isn't so hard to change 'svn info --changelist foo' into
> > > > 'svn info -R --changelist foo'.
> > >
> > > Er, you what?! If the user issues a command with '--changelist foo' as the only
> > > file selection criterion, then he/she wants to act on the files in that
> > > changelist. I can't see any other possible interpretation for 'svn info
> > > --changelist foo'. The same goes for all subcommands - I've just run through
> > > them all in my head.
> > >
> > >  From a user's point of view, I interpret '--changelist foo' as "act on the
> > > targets specified in changelist 'foo'", just the same as '--targets foo' means
> > > "act on the targets specified in file 'foo'". Any default recursion or
> > > non-recursion that a particular command assumes would come after that step, in
> > > my head.
> >
> > FWIW, it sounds like you both are saying it should work the same way.
> > The part that I suspect Julian was missing was that before you made
> > this change many of the subcommands  were essentially adding what was
> > in the changelist to what was in CWD which really does not make any
> > sense.  And clearly I think a user expects:
> >
> > svn info --changelist foo /path
> >
> > To show the info for the items in /path that belong to changelist foo.
> >  That is what Julian says it should do and that is what Mike just made
> > it do with these changes.
>
> No, it doesn't now, because Mike's changes made this mean "show info
> about everything that 'svn info /path' would show which is on
> changelist foo"; since info is by default depth=empty, that would be
> nothing.

Oh, right.  But maybe that command is just a bad example because when
I think about it that is probably what I would expect to happen.  I
only used svn info as an example because everyone else was.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: UI interpretation quetsions: --changelist + --depth

Posted by "C. Michael Pilato" <cm...@collab.net>.
Julian Foad wrote:
>>> No, it doesn't now, because Mike's changes made this mean "show info
>>> about everything that 'svn info /path' would show which is on
>>> changelist foo"; since info is by default depth=empty, that would be
>>> nothing.
> 
> I'm happy with it doing that: showing nothing. I don't see that as a 
> problem or particularly surprising.

[...]

> Therefore it looks to me like it would be a bad idea to change the 
> default depth of these commands. It would be unnecessarily surprising.

Just for clarity's sake, Julian, let's step back to the moment just before 
you wrote your original longish mail on this thread, but grant you the 
ability to foresee our various responses to it.  What mail would you now 
write?  (I got a little lost in your reply ... just trying to figure out 
where you stand at the moment.)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: UI interpretation quetsions: --changelist + --depth

Posted by Julian Foad <ju...@btopenworld.com>.
Ben Collins-Sussman wrote:
> I do think Julian has at least one valid point:  people familiar with
> perforce are going to find svn's definition of changelist a bit odd.
> In perforce, '--changelist foo' is a specific list of paths.  It
> doesn't matter where you're sitting in the working copy;  it's always
> the same complete list of paths.   It really does mean "act on this
> specific list of targets", rather than "do some recursion on some
> target and filter whatever you find".

Ah, yes, whereas a Subversion Changelist is not a distinct entity but just a 
name that is a property of one or more files.

Sorry, I'd forgotton that and thereby wrote a long mail heading in the wrong 
direction.

It doesn't matter in itself that we have a different concept than Perforce 
does: this is a different version control system.

However, be aware that the differences are considerable. A Perforce Changelist 
also has a log message and other metadata attached to it. And it can be pending 
or committed, like a Subversion Transaction. When Perforce users talk about a 
"changelist" they are often referring to what we would call the revision number 
of a committed change. And some of us would like to implement a client-side 
"pending commit" in Subversion in the medium term. That would be much more like 
a Perforce Pending Changelist.


> [paraphrasing: As that doesn't fit into Subversion's WC architecture,]
> Mike's implementation of 'make it a filter' is the only reasonable
> consistent behavior, I think.

Right. And that's OK. (Thanks for the reminder of why it's not like I thought 
it would be.)

Most of my previous post was about how a changelist specification should 
combine with other target specifications and other changelist specifications on 
the same command. Thinking it was a self-contained target specification, I 
suggested it should not combine with any other target specifications.

But now, the basic rule is that a command with '--changelist' specification(s) 
shall act on the intersection of the targets that the command would have acted 
on without any changelist specifications, and the targets in the union of all 
specified changelists. (We may want to disallow multiple changelist 
specifications in one command, for now.)

Except... There is the suggestion that a '--changelist' option should cause the 
default depth of any command to become infinity.

C. Michael Pilato wrote:
> David Glasser wrote:
>>> FWIW, it sounds like you both are saying it should work the same way.
>>> The part that I suspect Julian was missing was that before you made
>>> this change many of the subcommands  were essentially adding what was
>>> in the changelist to what was in CWD which really does not make any
>>> sense.  And clearly I think a user expects:
>>>
>>> svn info --changelist foo /path
>>>
>>> To show the info for the items in /path that belong to changelist foo.
>>>  That is what Julian says it should do and that is what Mike just made
>>> it do with these changes.

No, I didn't say it should do that.

>> No, it doesn't now, because Mike's changes made this mean "show info
>> about everything that 'svn info /path' would show which is on
>> changelist foo"; since info is by default depth=empty, that would be
>> nothing.

I'm happy with it doing that: showing nothing. I don't see that as a problem or 
particularly surprising.

> Hence, my suggestion that the presence of --changelist implies '--depth
> infinity' (unless --depth is also explicitly provided).  I *think* that
> will please the majority of folks, and I'm sorry I didn't think of it
> before.
> 
> Are there objections to my making exactly this change?  It should be an
> easy tweak to svn/main.c.

Until now we have restricted the default depth of approximately the following 
commands (looking at ones that accept "--recursive"):

   info
   list
   propXXX
   resolved
   revert

Changing the depth obviously only makes a difference to commands where a 
directory target is given. Commands "info", "list", and some of "propXXX" are 
harmless if unexpectedly acting on many members of a changelist. But "propdel 
dir1", "propset .", "resolved ." or "revert dir2" would be far from harmless.

Therefore it looks to me like it would be a bad idea to change the default 
depth of these commands. It would be unnecessarily surprising.

Regards,
- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: UI interpretation quetsions: --changelist + --depth

Posted by Ben Collins-Sussman <su...@red-bean.com>.
I do think Julian has at least one valid point:  people familiar with
perforce are going to find svn's definition of changelist a bit odd.
In perforce, '--changelist foo' is a specific list of paths.  It
doesn't matter where you're sitting in the working copy;  it's always
the same complete list of paths.   It really does mean "act on this
specific list of targets", rather than "do some recursion on some
target and filter whatever you find".

Of course, the reason the perforce implementation is even possible is
because the server is tracking every working copy and every file
'opened' for editing.  The list of changed paths is produced instantly
(i.e. no wc tree-walking) by consulting a server database.

So while I *like* this interpretation, there's no way svn can ever get
there unless we teach libsvn_wc to manage a central database of
changelists in ~/.subversion/.  Given our current libsvn_wc design,
Mike's implementation of 'make it a filter' is the only reasonable
consistent behavior, I think.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: UI interpretation quetsions: --changelist + --depth

Posted by "C. Michael Pilato" <cm...@collab.net>.
David Glasser wrote:
>> FWIW, it sounds like you both are saying it should work the same way.
>> The part that I suspect Julian was missing was that before you made
>> this change many of the subcommands  were essentially adding what was
>> in the changelist to what was in CWD which really does not make any
>> sense.  And clearly I think a user expects:
>>
>> svn info --changelist foo /path
>>
>> To show the info for the items in /path that belong to changelist foo.
>>  That is what Julian says it should do and that is what Mike just made
>> it do with these changes.
> 
> No, it doesn't now, because Mike's changes made this mean "show info
> about everything that 'svn info /path' would show which is on
> changelist foo"; since info is by default depth=empty, that would be
> nothing.

Hence, my suggestion that the presence of --changelist implies '--depth 
infinity' (unless --depth is also explicitly provided).  I *think* that will 
please the majority of folks, and I'm sorry I didn't think of it before.

Are there objections to my making exactly this change?  It should be an easy 
tweak to svn/main.c.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: UI interpretation quetsions: --changelist + --depth

Posted by David Glasser <gl...@davidglasser.net>.
On Jan 26, 2008 12:38 PM, Mark Phippard <ma...@gmail.com> wrote:
>
> On Jan 26, 2008 12:46 PM, Julian Foad <ju...@btopenworld.com> wrote:
> > Hi Mike and everyone!
> >
> > C. Michael Pilato wrote:
> > > Karl Fogel wrote:
> > >
> > >> Ummm, how about:
> > >>
> > >>   For most commands, '--changelist FOO' restricts the scope to items
> > >>   in the intersection of changelist FOO and the specified (or default)
> > >>   depth.  But for *some* commands, [...]
> > >>   '--changelist FOO' changes their non-infinity default depth to
> > >>   infinity, so that the changelist becomes the only restriction when
> > >>   no depth restriction is explicitly given.
> > >>
> > >> [...]
> > >>
> > >> I know people will cry "inconsistency", but I'm not sure consistency
> > >> is a major factor in good UI anyway.  Meeting intuitive expectations
> > >> is more important.
> > >>
> > >> -Karl
> > >
> > > [...] perhaps we should just roll with the
> > > easy-to-explain "changelist is always a filter, period" explanation.  After
> > > all, it really isn't so hard to change 'svn info --changelist foo' into
> > > 'svn info -R --changelist foo'.
> >
> > Er, you what?! If the user issues a command with '--changelist foo' as the only
> > file selection criterion, then he/she wants to act on the files in that
> > changelist. I can't see any other possible interpretation for 'svn info
> > --changelist foo'. The same goes for all subcommands - I've just run through
> > them all in my head.
> >
> >  From a user's point of view, I interpret '--changelist foo' as "act on the
> > targets specified in changelist 'foo'", just the same as '--targets foo' means
> > "act on the targets specified in file 'foo'". Any default recursion or
> > non-recursion that a particular command assumes would come after that step, in
> > my head.
>
> FWIW, it sounds like you both are saying it should work the same way.
> The part that I suspect Julian was missing was that before you made
> this change many of the subcommands  were essentially adding what was
> in the changelist to what was in CWD which really does not make any
> sense.  And clearly I think a user expects:
>
> svn info --changelist foo /path
>
> To show the info for the items in /path that belong to changelist foo.
>  That is what Julian says it should do and that is what Mike just made
> it do with these changes.

No, it doesn't now, because Mike's changes made this mean "show info
about everything that 'svn info /path' would show which is on
changelist foo"; since info is by default depth=empty, that would be
nothing.

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org