Re: svn commit: r29672 - trunk/subversion/svn

[Lieven Govaerts <sv...@mobsol.be>]

cmpilato@tigris.org wrote:
> Author: cmpilato
> Date: Mon Mar  3 08:46:44 2008
> New Revision: 29672
> 
> Log:
> Finish issue #3108 by raising an error for 'svn cl FOO' or svn cl
> --remove' (with no explicit target path).
> 
> NOTE:  I'm not sure I fully agree with this course of action, but I
> don't disagree enough stand in the way of correcting the general UI
> problem of running a command only to see nothing happening.
> 
> * subversion/svn/changelist-cmd.c
>   (svn_cl__changelist): Don't add the implicit dot target; instead,
>     error if no paths are provided.
> 

Hm. I'm not in favor of this change.

After r29671 the output of changelist is:

$ svn cl blabla
Skipped '.'

I think this makes perfect sense, as we use the same notification to 
indicate lack of directory support in changelist.

If in the future we add dir support, do we still want the user to 
provide the explicit '.' target? I'd think not, as we don't expect it 
for most of the other commands.

So now we have two different ways to tell the user that directories are 
not supported, either by the 'Skipped' notification, or with this 
exception. I find this is more confusing than helpful.

Lieven

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

Re: svn commit: r29672 - trunk/subversion/svn

[sv...@mobsol.be]

Quoting Julian Foad <ju...@btopenworld.com>:

> Lieven Govaerts wrote:
..
>
> Consider "svn cl --recursive blabla DIR" which currently means add all files
> under DIR. Is it sensible to have a default target of "." in this case?
> Imagine
> if changelists do eventually support directories. Maybe the default action on
> a
> specified directory will be to add it as depth==0, or maybe as
> depth==infinite.
> Is it sensible to have a default target of "." in any of these cases?
>

Well, I think yes, as this is the first thing I tried to add a property change
on the root dir of my working copy to a changelist.

> Save the default syntax for some (as yet unknown) purpose that really
> warrants
> it. For example, maybe we will eventually have a way to "view" or "activate"
> (in some sense) a changelist, and maybe one of those actions will become very
> common. Then it might make sense to make "svn cl blabla" mean that common
> action.
>
Ok.

>
> > So now we have two different ways to tell the user that directories are
> > not supported, either by the 'Skipped' notification, or with this
> > exception. I find this is more confusing than helpful.
>
> No, we don't. The exception "Not enough arguments provided" has nothing to do
> with whether directories are supported.
>
> I hope that makes sense to you now.
>
Well, I don't agree with everything you said, but given this discussion is about
the best way to tell a user that the we haven't really fixed what 'svn cl
blabla' should do I have no problem removing my -0 for now. We keep our options
open and continue the discussion as soon as we've found out how users are using
the changelist feature.

Lieven


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.


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

Re: svn commit: r29672 - trunk/subversion/svn

[Julian Foad <ju...@btopenworld.com>]

Lieven Govaerts wrote:
> cmpilato@tigris.org wrote:
> 
>> Author: cmpilato
>> Date: Mon Mar  3 08:46:44 2008
>> New Revision: 29672
>>
>> Log:
>> Finish issue #3108 by raising an error for 'svn cl FOO' or svn cl
>> --remove' (with no explicit target path).
>>
>> NOTE:  I'm not sure I fully agree with this course of action, but I
>> don't disagree enough stand in the way of correcting the general UI
>> problem of running a command only to see nothing happening.
>>
>> * subversion/svn/changelist-cmd.c
>>   (svn_cl__changelist): Don't add the implicit dot target; instead,
>>     error if no paths are provided.
>>
> 
> Hm. I'm not in favor of this change.

(After this change, r29672:

$ svn cl blabla
svn: Try 'svn help' for more info
svn: Not enough arguments provided
)


> After r29671 the output of changelist is:

(was)
> 
> $ svn cl blabla
> Skipped '.'
> 
> I think this makes perfect sense, as we use the same notification to 
> indicate lack of directory support in changelist.
> 
> If in the future we add dir support, do we still want the user to 
> provide the explicit '.' target? I'd think not, as we don't expect it 
> for most of the other commands.

Yes we do want to require an explicit target. I can't see the use case of "Add 
the current directory to this changelist" to be anywhere near common enough to 
warrant it being the default action of the "changelist" command.

Some of our commands default to acting on the current directory because it's 
very commonly wanted and a "safe" default (like "svn status"). There's no need 
for other commands to take the same or any default where it's not very commonly 
useful and safe to do so.

Consider "svn cl --recursive blabla DIR" which currently means add all files 
under DIR. Is it sensible to have a default target of "." in this case? Imagine 
if changelists do eventually support directories. Maybe the default action on a 
specified directory will be to add it as depth==0, or maybe as depth==infinite. 
Is it sensible to have a default target of "." in any of these cases?

Save the default syntax for some (as yet unknown) purpose that really warrants 
it. For example, maybe we will eventually have a way to "view" or "activate" 
(in some sense) a changelist, and maybe one of those actions will become very 
common. Then it might make sense to make "svn cl blabla" mean that common action.


> So now we have two different ways to tell the user that directories are 
> not supported, either by the 'Skipped' notification, or with this 
> exception. I find this is more confusing than helpful.

No, we don't. The exception "Not enough arguments provided" has nothing to do 
with whether directories are supported.

I hope that makes sense to you now.

- Julian

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