Re: shelve is (surprisingly) sensitive to working directory

[Julian Foad <ju...@apache.org>]

James McCoy wrote:
> While messing around with things recently, I decided to try out shelve
> to save some of my pending changes.

Thanks for trying it!

[...]> jamessan@freya:~/src/apache.org/subversion/trunk$ 
./subversion/svn/svn shelve demo
> C         /home/jamessan/src/apache.org/subversion/subversion/libsvn_client/shelve.c
>>          rejected hunk @@ -216,7 +216,6 @@
> shelved 'demo'
> ]]]
> 
> Notice that a conflict is shown and the full path is incorrect (missing
> /trunk/).  It looks like the relative path from my working directory was
> appended to the working copy root, which is clearly wrong.

Ugh. Thanks for finding this.

Fixed in http://svn.apache.org/r1819804

Please could you check it.

The working directory still needs to be within the WC for all the 
shelving commands. When you specify a path ("svn shelve foo 
path-to-wc/...") this is counter-intuitive, and probably should be 
changed: it should check all given paths (default: ".") are in the same 
WC and use that WC.

The other commands, "svn unshelve foo", "svn shelve --delete foo" and 
"svn shelves" don't currently accept paths but maybe they could.

When listing shelves, maybe we should list only those that affect paths 
entirely within the given path(s), or only those that affect at least 
some paths within the given path(s). E.g. (just thinking out loud):

[[[
$ svn shelves
--- shelves entirely within '.':
foo
--- shelves partially within '.':
bar
--- 13 shelves are outside '.'; specify '--all' to list them
]]]

It would be good to hear any thoughts you have about how to improve it.

- Julian

Re: shelve is (surprisingly) sensitive to working directory

[Branko Čibej <br...@apache.org>]

On 04.01.2018 05:28, James McCoy wrote

>> The other commands, "svn unshelve foo", "svn shelve --delete foo" and "svn
>> shelves" don't currently accept paths but maybe they could.
> I don't really see the use for that.  I referenced git's stash
> implementation, since I somewhat regularly use that, and was surprised
> to learn that it supports specifying paths when creating and listing
> stashes.

This is a trap we should steer well clear of. Git's way of handling its
checkout is fundamentally different from Subversion's handling of the
working copy: except for clone, you can't do anything outside the
checkout tree, and any command within it affects the whole tree.

Instead of looking at Git's semantics, it would be better to see how,
e.g., 'svn diff' works with regard to the default path and path
arguments, and make shelve consistent with that.

-- Brane

Re: shelve is (surprisingly) sensitive to working directory

[Julian Foad <ju...@apache.org>]

James McCoy wrote:
> Much better, thanks.

Thanks for checking that.

[...snip...]

Thanks for your other thoughts.

> The only other improvement that comes to mind is for "svn shelves" to
> show the (WC or curdir) relative file path.  Currently, it just shows the file
> name with the common parent directories elided.
[...]
>   libsvn_client/shelve.c |    1 +
>   svn/shelve-cmd.c       |    1 +
[...]
Good idea. Done in http://svn.apache.org/r1820044, relative to WC root. 
(Relative to CWD would be nice, but harder.)

- Julian

Re: shelve is (surprisingly) sensitive to working directory

[James McCoy <ja...@jamessan.com>]

On Tue, Jan 02, 2018 at 12:06:47PM +0000, Julian Foad wrote:
> James McCoy wrote:
> Fixed in http://svn.apache.org/r1819804
> 
> Please could you check it.

Much better, thanks.

> The working directory still needs to be within the WC for all the shelving
> commands.

That seems like a reasonable restriction for an initial, experimental
release especially if it's documented.  The intention is to get feedback
to help shape the remaining functionality.

> When you specify a path ("svn shelve foo path-to-wc/...") this is
> counter-intuitive, and probably should be changed: it should check all given
> paths (default: ".") are in the same WC and use that WC.

Yes, this would be better.

> The other commands, "svn unshelve foo", "svn shelve --delete foo" and "svn
> shelves" don't currently accept paths but maybe they could.

I don't really see the use for that.  I referenced git's stash
implementation, since I somewhat regularly use that, and was surprised
to learn that it supports specifying paths when creating and listing
stashes.

> When listing shelves, maybe we should list only those that affect paths
> entirely within the given path(s), or only those that affect at least some
> paths within the given path(s). E.g. (just thinking out loud):

I would wait for more feedback before going down that route.

> It would be good to hear any thoughts you have about how to improve it.

The only other improvement that comes to mind is for "svn shelves" to
show the (WC or curdir) relative file path.  Currently, it just shows the file
name with the common parent directories elided.

[[[
jamessan@freya:~/src/apache.org/subversion/trunk$ ./subversion/svn/svn shelves
single                              2 mins old        636 bytes    1 paths changed

 shelve.c |    1 +
 1 file changed, 1 insertion(+)

multi                               0 mins old       1107 bytes    2 paths changed

 libsvn_client/shelve.c |    1 +
 svn/shelve-cmd.c       |    1 +
 2 files changed, 2 insertions(+)
]]]

The "single" shelf should be showing subversion/libsvn_client/shelve.c
as the path, while "multi" should be showing that and
subversion/svn/shelve-cmd.c.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB