You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.co.il> on 2008/02/23 14:32:53 UTC
[PATCH] Issue #3108: "svn changelist clname" does nothin
[[[
Fix issue #3108: "svn changelist clname" does nothing.
* subversion/svn/changelist-cmd.c
(svn_cl__changelist):
Require --changelist or an explicit path argument.
* subversion/tests/cmdline/changelist_tests.py
(changelist_without_arguments): New test.
(test_list): Run it.
]]]
The new test doesn't intend to check that changelist renaming ('svn cl
foo --cl bar') works correctly---only that it is syntactically allowed.
Daniel
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
[ An update for the records: ]
Daniel Shahaf wrote on Fri, 29 Feb 2008 at 19:08 +0200:
> Two patches attached, implementing:
>
> * 'svn cl': default to 'svn_depth_infinity'
As I understand the situation, no one thinks now that this change should
be made.
> * 'svn cl': generate 'skip' notifications on directories
>
cmpilato committed this patch in r29671, following discussion here:
http://svn.haxx.se/dev/archive-2008-03/0030.shtml
> The three patches (these two and the one I just posted elsethread)
The third patch (no implicit dot) is made unnecessary by r29672:
http://svn.haxx.se/dev/archive-2008-03/0048.shtml
>
Thanks all,
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
[ An update for the records: ]
Daniel Shahaf wrote on Fri, 29 Feb 2008 at 19:08 +0200:
> Two patches attached, implementing:
>
> * 'svn cl': default to 'svn_depth_infinity'
As I understand the situation, no one thinks now that this change should
be made.
> * 'svn cl': generate 'skip' notifications on directories
>
cmpilato committed this patch in r29671, following discussion here:
http://svn.haxx.se/dev/archive-2008-03/0030.shtml
> The three patches (these two and the one I just posted elsethread)
The need for third patch is removed by r29672:
http://svn.haxx.se/dev/archive-2008-03/0048.shtml
>
Thanks all,
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Daniel Shahaf wrote on Wed, 27 Feb 2008 at 18:07 +0200:
> David Glasser wrote on Tue, 26 Feb 2008 at 15:34 -0800:
>> On Tue, Feb 26, 2008 at 3:16 PM, Daniel Shahaf <d....@daniel.shahaf.co.il>
>> wrote:
>>> David Glasser wrote on Tue, 26 Feb 2008 at 13:10 -0800:
>>> > Hmm. I kind of feel like "svn changelist" should default to
>>> > svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
>>> > --cl old" lets you do a rename, without need an extra "-R"
>>>
>>> I think svn_depth_empty would be a better default if dirs can be members
>>> of changelists (otherwise, every changelist-operation on the dir applies
>>> to its children too). I agree svn_depth_infinite is a better default if
>>> dirs cannot be members of changelists.
>>
>> They can't be at the moment.
>>
>> I'm OK with, if a future version decides to allow directories on
>> changelists, making people use "svn cl --depth=empty name dir" to put
>> the directory on it (or -N).
>>
...
>>> > (If this change doesn't happen, perhaps a warning (ie, notification)
>>> > that a directory is being skipped would be useful.)
>>> >
>>>
>>> +1 (moral support).
>>>
>
> I will prepare a new patch, then, implementing 'svn_depth_infinite' as
> default and 'skip' notifications.
>
Two patches attached, implementing:
* 'svn cl': default to 'svn_depth_infinity'
* 'svn cl': generate 'skip' notifications on directories [1]
The three patches (these two and the one I just posted elsethread)
should be mutually independent, and I believe they can be applied in
any order, but I have not tested them separately.
[1] Notes on the "'skip' notifications" patch:
* I added infrastructure to changelist_tests:1 to parse 'skip' output,
but it is not used yet.
Daniel
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
David Glasser wrote on Wed, 27 Feb 2008 at 09:31 -0800:
> On Wed, Feb 27, 2008 at 9:14 AM, Julian Foad <ju...@btopenworld.com> wrote:
>> I recommend the "changelist" command not assume an implicit "." ever, which is
>> in accordance with its current help text but against its current implementation
>> (trunk@29608M).
>
> +1
>
[[[
In the commandline client, require at least one path argument to
'svn changelist', making it consistent with its help string.
Part of issue #3108.
* subversion/svn/changelist-cmd.c
(svn_cl__changelist):
Error out unconditionally if no explicit target was given.
* subversion/tests/cmdline/changelist_tests.py
(changelist_without_arguments): New test.
(test_list): Run it.
]]]
Daniel
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by David Glasser <gl...@davidglasser.net>.
On Wed, Feb 27, 2008 at 9:14 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Daniel Shahaf wrote:
> > David Glasser wrote on Tue, 26 Feb 2008 at 15:34 -0800:
>
> >> On Tue, Feb 26, 2008 at 3:16 PM, Daniel Shahaf wrote:
> >>> David Glasser wrote on Tue, 26 Feb 2008 at 13:10 -0800:
> >>> > Hmm. I kind of feel like "svn changelist" should default to
> >>> > svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
> >>> > --cl old" lets you do a rename, without need an extra "-R"
> [...]
>
> >>> > and (b)
> >>> > "svn cl clname directory" will add the directory's recursive contents
> >>> > to clname instead of just doing *nothing*.
> [...]
>
> > Do you think the directory should be given explicitly in this usage?
> > Otherwise, plain 'svn cl NAME' would recursively remove all files in the
> > current directory from their changelists.
>
> A TARGET argument should be required in all commands except where an implicit
> "." is demonstrably useful enough to be worth the saving of only two characters
> of typing, and where it's a safe default. (Unfortunately, I think some svn
> commands already assume an implicit "." where it's not very useful.)
>
> I recommend the "changelist" command not assume an implicit "." ever, which is
> in accordance with its current help text but against its current implementation
> (trunk@29608M).
+1
--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
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Shahaf wrote:
> David Glasser wrote on Tue, 26 Feb 2008 at 15:34 -0800:
>> On Tue, Feb 26, 2008 at 3:16 PM, Daniel Shahaf wrote:
>>> David Glasser wrote on Tue, 26 Feb 2008 at 13:10 -0800:
>>> > Hmm. I kind of feel like "svn changelist" should default to
>>> > svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
>>> > --cl old" lets you do a rename, without need an extra "-R"
[...]
>>> > and (b)
>>> > "svn cl clname directory" will add the directory's recursive contents
>>> > to clname instead of just doing *nothing*.
[...]
> Do you think the directory should be given explicitly in this usage?
> Otherwise, plain 'svn cl NAME' would recursively remove all files in the
> current directory from their changelists.
A TARGET argument should be required in all commands except where an implicit
"." is demonstrably useful enough to be worth the saving of only two characters
of typing, and where it's a safe default. (Unfortunately, I think some svn
commands already assume an implicit "." where it's not very useful.)
I recommend the "changelist" command not assume an implicit "." ever, which is
in accordance with its current help text but against its current implementation
(trunk@29608M).
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
David Glasser wrote on Tue, 26 Feb 2008 at 15:34 -0800:
> On Tue, Feb 26, 2008 at 3:16 PM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
>> David Glasser wrote on Tue, 26 Feb 2008 at 13:10 -0800:
>> > Hmm. I kind of feel like "svn changelist" should default to
>> > svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
>> > --cl old" lets you do a rename, without need an extra "-R"
>>
>> I think svn_depth_empty would be a better default if dirs can be members
>> of changelists (otherwise, every changelist-operation on the dir applies
>> to its children too). I agree svn_depth_infinite is a better default if
>> dirs cannot be members of changelists.
>
> They can't be at the moment.
>
> I'm OK with, if a future version decides to allow directories on
> changelists, making people use "svn cl --depth=empty name dir" to put
> the directory on it (or -N).
>
>> > and (b)
>> > "svn cl clname directory" will add the directory's recursive contents
>> > to clname instead of just doing *nothing*.
>> >
>>
>> If all the dir's children should be in a changelist, why not pass that
>> dir as an argument on the commandline, avoiding changelists altogether?
>
> Because they might not be committing for a while...
>
Okay.
Do you think the directory should be given explicitly in this usage?
Otherwise, plain 'svn cl NAME' would recursively remove all files in the
current directory from their changelists.
>> > (If this change doesn't happen, perhaps a warning (ie, notification)
>> > that a directory is being skipped would be useful.)
>> >
>>
>> +1 (moral support).
>>
I will prepare a new patch, then, implementing 'svn_depth_infinite' as
default and 'skip' notifications.
Thanks for the review, David.
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by David Glasser <gl...@davidglasser.net>.
On Tue, Feb 26, 2008 at 3:16 PM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> David Glasser wrote on Tue, 26 Feb 2008 at 13:10 -0800:
>
> > 2008/2/23 Daniel Shahaf <d....@daniel.shahaf.co.il>:
> >> [[[
> >> Fix issue #3108: "svn changelist clname" does nothing.
> >>
> >> * subversion/svn/changelist-cmd.c
> >> (svn_cl__changelist):
> >> Require --changelist or an explicit path argument.
> >>
> >> * subversion/tests/cmdline/changelist_tests.py
> >> (changelist_without_arguments): New test.
> >> (test_list): Run it.
> >> ]]]
> >>
> >> The new test doesn't intend to check that changelist renaming ('svn cl
> >> foo --cl bar') works correctly---only that it is syntactically allowed.
> >
> > Um, and when you say "the new test doesn't intend to check", you also
> > mean "with this patch, svn cl foo --cl bar still does nothing", right?
> > (Because that's what I'm seeing, and it's kind of unfortunately.)
> >
>
> Yes, it still does nothing and still exits successfully, except that it
> requires an explicit path argument: 'svn cl foo --cl bar -R .' now,
> instead of 'svn cl foo --cl bar -R' before. The .-less invocation would
> not throw an error with the patch; it would do nothing silently.
>
>
> > Hmm. I kind of feel like "svn changelist" should default to
> > svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
> > --cl old" lets you do a rename, without need an extra "-R"
>
> I think svn_depth_empty would be a better default if dirs can be members
> of changelists (otherwise, every changelist-operation on the dir applies
> to its children too). I agree svn_depth_infinite is a better default if
> dirs cannot be members of changelists.
They can't be at the moment.
I'm OK with, if a future version decides to allow directories on
changelists, making people use "svn cl --depth=empty name dir" to put
the directory on it (or -N).
> > and (b)
> > "svn cl clname directory" will add the directory's recursive contents
> > to clname instead of just doing *nothing*.
> >
>
> If all the dir's children should be in a changelist, why not pass that
> dir as an argument on the commandline, avoiding changelists altogether?
Because they might not be committing for a while...
--dave
> > (If this change doesn't happen, perhaps a warning (ie, notification)
> > that a directory is being skipped would be useful.)
> >
>
> +1 (moral support).
>
> Daniel
>
> > --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
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
David Glasser wrote on Tue, 26 Feb 2008 at 13:10 -0800:
> 2008/2/23 Daniel Shahaf <d....@daniel.shahaf.co.il>:
>> [[[
>> Fix issue #3108: "svn changelist clname" does nothing.
>>
>> * subversion/svn/changelist-cmd.c
>> (svn_cl__changelist):
>> Require --changelist or an explicit path argument.
>>
>> * subversion/tests/cmdline/changelist_tests.py
>> (changelist_without_arguments): New test.
>> (test_list): Run it.
>> ]]]
>>
>> The new test doesn't intend to check that changelist renaming ('svn cl
>> foo --cl bar') works correctly---only that it is syntactically allowed.
>
> Um, and when you say "the new test doesn't intend to check", you also
> mean "with this patch, svn cl foo --cl bar still does nothing", right?
> (Because that's what I'm seeing, and it's kind of unfortunately.)
>
Yes, it still does nothing and still exits successfully, except that it
requires an explicit path argument: 'svn cl foo --cl bar -R .' now,
instead of 'svn cl foo --cl bar -R' before. The .-less invocation would
not throw an error with the patch; it would do nothing silently.
> Hmm. I kind of feel like "svn changelist" should default to
> svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
> --cl old" lets you do a rename, without need an extra "-R"
I think svn_depth_empty would be a better default if dirs can be members
of changelists (otherwise, every changelist-operation on the dir applies
to its children too). I agree svn_depth_infinite is a better default if
dirs cannot be members of changelists.
> and (b)
> "svn cl clname directory" will add the directory's recursive contents
> to clname instead of just doing *nothing*.
>
If all the dir's children should be in a changelist, why not pass that
dir as an argument on the commandline, avoiding changelists altogether?
> (If this change doesn't happen, perhaps a warning (ie, notification)
> that a directory is being skipped would be useful.)
>
+1 (moral support).
Daniel
> --dave
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #3108: "svn changelist clname" does nothin
Posted by David Glasser <gl...@davidglasser.net>.
2008/2/23 Daniel Shahaf <d....@daniel.shahaf.co.il>:
> [[[
> Fix issue #3108: "svn changelist clname" does nothing.
>
> * subversion/svn/changelist-cmd.c
> (svn_cl__changelist):
> Require --changelist or an explicit path argument.
>
> * subversion/tests/cmdline/changelist_tests.py
> (changelist_without_arguments): New test.
> (test_list): Run it.
> ]]]
>
> The new test doesn't intend to check that changelist renaming ('svn cl
> foo --cl bar') works correctly---only that it is syntactically allowed.
Um, and when you say "the new test doesn't intend to check", you also
mean "with this patch, svn cl foo --cl bar still does nothing", right?
(Because that's what I'm seeing, and it's kind of unfortunately.)
Hmm. I kind of feel like "svn changelist" should default to
svn_depth_infinite rather than svn_depth_empty. Then (a) "svn cl new
--cl old" lets you do a rename, without need an extra "-R" and (b)
"svn cl clname directory" will add the directory's recursive contents
to clname instead of just doing *nothing*.
(If this change doesn't happen, perhaps a warning (ie, notification)
that a directory is being skipped would be useful.)
--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