A rational svn shelve/checkpoint CLI

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

A rational svn shelve/checkpoint CLI

Julian Foad-5
Towards a more rational shelving-with-checkpoints CLI.

   svn shelf-{save,shelve,unshelve,shelves,log,drop,diff}
     [SHELF_SPEC...] [PATH...]

SHELF_SPEC:

   --shelf=[SHELF][.VERSION]
   @[SHELF][.VERSION]

     Specify shelf SHELF (name, 'all', 'newest'), version VERSION
     (number, date, 'all', 'newest'). Defaults are per command.

   --version=VERSION

     Specify the default version for shelf specs in the command.

Aliases:

   svn save            -> svn shelf-save
   svn shelve          -> svn shelf-shelve
   svn unshelve        -> svn shelf-unshelve
   svn shelves         -> svn shelf-shelves

Main commands:

   svn save | shelf-save      [@SHELF] [PATH...]    # save only
   svn shelve | shelf-shelve  [@SHELF] [PATH...]    # save & revert

     defaults: SHELF:newest; PATH:'.'
     Save/Shelve to shelf SHELF the changes within a PATH.

   svn unshelve | shelf-unshelve  [@[SHELF][.VERSION]...] [PATH...]

     defaults: SHELF:newest; VERSION:newest; PATH:'.'
     Unshelve each shelf-version SHELF.VERSION
     just the paths within PATH...
     (Warn if that omits other paths in the shelf-version.)

Ancillary commands:

   svn shelves | shelf-shelves  [@SHELF...] [PATH...]

     defaults: SHELF:all; PATH:'.'
     List each shelf SHELF that has changes within a PATH.
     Same as 'shelf-log --version=newest ...'.

   svn shelf-log      [@[SHELF][.VERSION]...] [PATH...]

     defaults: SHELF:newest; VERSION:all; PATH:'.'
     List each version in a shelf SHELF that has changes inside a PATH.

   svn shelf-drop     [@SHELF...]

     defaults: SHELF:newest
     Delete each shelf SHELF.

   svn shelf-diff     [@[SHELF][.VERSION]...] [PATH...]

     defaults: SHELF:newest; VERSION:newest; PATH:'.'
     Print a diff of the changes inside PATH... in each shelf-version.
     (Note: not the difference between two shelves or versions.)


Thoughts:

The use of a distinctive shelf specifier is intended to avoid the
ambiguity in the first optional argument to a syntax like "svn shelve
[NAME] [PATH...]". There are some other possible ways to resolve the
ambiguity, such as interpreting the first optional argument as NAME
unless it contains a '/' or '\' (if we don't allow '/' or '\' in NAME).

Using one-word subcommands like "shelf-drop" rather than "shelf drop" or
"shelf --delete" is intended to be better for consistency, for the help
system, and for extending existing scripting that assumes one-word
subcommands (the bash-completion script is one example). But note the
ugly repetition in "shelf-shelve", "shelve-unshelve", and "shelf-shelves".

A common syntax for referring to a particular shelved change is helpful.
"--shelf=" is OK for a start but the repetition of "shelf" here after
the subcommand name is ugly again, hence suggestion of "@" instead,
although using "@" to introduce the name rather than the version is
inconsistent with peg revision specifiers.

Perhaps we can take this further and achieve a syntax like "svn log
--shelf=NAME" and "svn diff --shelf=NAME[@VERSION]" in which a
shelf-spec is used with a regular existing subcommand, eliminating the
repetition in those commands. Then 'svn shelve' and 'unshelve' and
'shelves' become primary subcommands, but they still need a shelf specifier.


- Julian
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Branko Čibej
On 08.01.2018 14:31, Julian Foad wrote:

> Towards a more rational shelving-with-checkpoints CLI.
>
>   svn shelf-{save,shelve,unshelve,shelves,log,drop,diff}
>     [SHELF_SPEC...] [PATH...]
>
> SHELF_SPEC:
>
>   --shelf=[SHELF][.VERSION]
>   @[SHELF][.VERSION]
>
>     Specify shelf SHELF (name, 'all', 'newest'), version VERSION
>     (number, date, 'all', 'newest'). Defaults are per command.
>
>   --version=VERSION
>
>     Specify the default version for shelf specs in the command.
>
> Aliases:
>
>   svn save            -> svn shelf-save
>   svn shelve          -> svn shelf-shelve
>   svn unshelve        -> svn shelf-unshelve
>   svn shelves         -> svn shelf-shelves


This naming doesn't make any sense to me. Using a word root in the same
command as both a noun and a verb runs counter to my notions of proper
grammar. It doesn't look right. The following would be better:

    shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
shelf-diff.[*]

Adding a common prefix to existing commands without renaming the
commands doesn't always make sense.

[*] It would be even better without the dash: 'shelf save', 'shelf
restore', and so on; but I realise that would involve either custom
command interpretation or some deep surgery in our command-line parser.

-- Brane

Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Julian Foad-5
Branko Čibej wrote:

> On 08.01.2018 14:31, Julian Foad wrote:
>> Towards a more rational shelving-with-checkpoints CLI.
>>
>>    svn shelf-{save,shelve,unshelve,shelves,log,drop,diff}
>>      [SHELF_SPEC...] [PATH...]
>>
>> SHELF_SPEC:
>>
>>    --shelf=[SHELF][.VERSION]
>>    @[SHELF][.VERSION]
>>
>>      Specify shelf SHELF (name, 'all', 'newest'), version VERSION
>>      (number, date, 'all', 'newest'). Defaults are per command.
>>
>>    --version=VERSION
>>
>>      Specify the default version for shelf specs in the command.
>>
>> Aliases:
>>
>>    svn save            -> svn shelf-save
>>    svn shelve          -> svn shelf-shelve
>>    svn unshelve        -> svn shelf-unshelve
>>    svn shelves         -> svn shelf-shelves
>
>
> This naming doesn't make any sense to me. Using a word root in the same
> command as both a noun and a verb runs counter to my notions of proper
> grammar. It doesn't look right. The following would be better:
>
>      shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
> shelf-diff.[*]

Thanks for your thoughts.

You missed out the 'shelve' command (which is like 'save' but reverts
the modifications) which is probably the most difficult one to
accommodate. What do you propose for that? If you intend 'save' to
include the revert then what do propose for the form that doesn't revert
(that is, form that creates another save-point/check-point)?

I considered using the word 'restore'. It is fine in the context of
'shelf-restore' but because of considering aliases I went for 'unshelve'
as the better one-word name, and let the two-word construction be ugly.
I also considered 'svn unshelve' as an alias with 'svn shelf-restore' as
the two-word form.

Similarly for 'shelves' vs. 'list', with an additional consideration
here that 'svn list' is already a command with a different meaning, a
meaning that could also usefully be applied to a shelf.

> Adding a common prefix to existing commands without renaming the
> commands doesn't always make sense.

Totally agree.

> [*] It would be even better without the dash: 'shelf save', 'shelf
> restore', and so on; but I realise that would involve either custom
> command interpretation or some deep surgery in our command-line parser.

I already have an implementation without the dash ('shelf save', etc.)
working on the 'shelve-checkpoint' branch. It suffers from the problems
I mentioned towards the end of my mail. I agree it would be better from
a human perspective. The problems of parsing and 'help' display could be
overcome, but I don't think the work to do so is justified at present.

- Julian

Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Stefan Sperling-9
On Mon, Jan 08, 2018 at 02:34:31PM +0000, Julian Foad wrote:

> > >    svn save            -> svn shelf-save
> > >    svn shelve          -> svn shelf-shelve
> > >    svn unshelve        -> svn shelf-unshelve
> > >    svn shelves         -> svn shelf-shelves
> >
> >
> > This naming doesn't make any sense to me. Using a word root in the same
> > command as both a noun and a verb runs counter to my notions of proper
> > grammar. It doesn't look right. The following would be better:
> >
> >      shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
> > shelf-diff.[*]
>
> Thanks for your thoughts.
>
> You missed out the 'shelve' command (which is like 'save' but reverts the
> modifications) which is probably the most difficult one to accommodate. What
> do you propose for that? If you intend 'save' to include the revert then
> what do propose for the form that doesn't revert (that is, form that creates
> another save-point/check-point)?
>
> I considered using the word 'restore'. It is fine in the context of
> 'shelf-restore' but because of considering aliases I went for 'unshelve' as
> the better one-word name, and let the two-word construction be ugly. I also
> considered 'svn unshelve' as an alias with 'svn shelf-restore' as the
> two-word form.
>
> Similarly for 'shelves' vs. 'list', with an additional consideration here
> that 'svn list' is already a command with a different meaning, a meaning
> that could also usefully be applied to a shelf.

How about a combination of new subcommands for actions which
tranfer working copy state from/to the shelf (used frequently),
and just one subcommand with options for management of the
shelf itself (used less frequently), like:

        svn shelve
        svn unshelve
        svn shelf --list
        svn shelf --drop
        svn shelf --diff/--show
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Julian Foad-5
Stefan Sperling wrote:

> On Mon, Jan 08, 2018 at 02:34:31PM +0000, Julian Foad wrote:
>>>>     svn save            -> svn shelf-save
>>>>     svn shelve          -> svn shelf-shelve
>>>>     svn unshelve        -> svn shelf-unshelve
>>>>     svn shelves         -> svn shelf-shelves
>>>
>>>
>>> This naming doesn't make any sense to me. Using a word root in the same
>>> command as both a noun and a verb runs counter to my notions of proper
>>> grammar. It doesn't look right. The following would be better:
>>>
>>>       shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
>>> shelf-diff.[*]
>>
>> Thanks for your thoughts.
>>
>> You missed out the 'shelve' command (which is like 'save' but reverts the
>> modifications) which is probably the most difficult one to accommodate. What
>> do you propose for that? If you intend 'save' to include the revert then
>> what do propose for the form that doesn't revert (that is, form that creates
>> another save-point/check-point)?
>>
>> I considered using the word 'restore'. It is fine in the context of
>> 'shelf-restore' but because of considering aliases I went for 'unshelve' as
>> the better one-word name, and let the two-word construction be ugly. I also
>> considered 'svn unshelve' as an alias with 'svn shelf-restore' as the
>> two-word form.
>>
>> Similarly for 'shelves' vs. 'list', with an additional consideration here
>> that 'svn list' is already a command with a different meaning, a meaning
>> that could also usefully be applied to a shelf.
>
> How about a combination of new subcommands for actions which
> tranfer working copy state from/to the shelf (used frequently),
> and just one subcommand with options for management of the
> shelf itself (used less frequently), like:
>
> svn shelve
> svn unshelve
> svn shelf --list
> svn shelf --drop
> svn shelf --diff/--show

That's a totally reasonable approach, and is similar to (not exactly the
same as) what's currently implemented on trunk and on shelve-checkpoint
branch.

FWIW you missed out 'svn save' (create a checkpoint, don't revert from WC).

The discussion of syntax for referring to a particular shelf and version
is perhaps the more interesting part of the whole discussion; I would
welcome any thoughts on that part.

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Stefan Sperling-9
On Mon, Jan 08, 2018 at 03:29:45PM +0000, Julian Foad wrote:

> > How about a combination of new subcommands for actions which
> > tranfer working copy state from/to the shelf (used frequently),
> > and just one subcommand with options for management of the
> > shelf itself (used less frequently), like:
> >
> > svn shelve
> > svn unshelve
> > svn shelf --list
> > svn shelf --drop
> > svn shelf --diff/--show
>
> That's a totally reasonable approach, and is similar to (not exactly the
> same as) what's currently implemented on trunk and on shelve-checkpoint
> branch.
>
> FWIW you missed out 'svn save' (create a checkpoint, don't revert from WC).

That could be 'svn shelve --keep'

> The discussion of syntax for referring to a particular shelf and version is
> perhaps the more interesting part of the whole discussion; I would welcome
> any thoughts on that part.

We could assign numbers to them, display those in 'svn shelf --list',
and let all other shelf-manipulating options expect a number.

Additionally, or instead, we could have user-provided shelf entry names
(UTF-8 strings), and allow them to be used for identification.
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Julian Foad-5
Stefan Sperling wrote:

>>> svn shelve
>>> svn unshelve
>>> svn shelf --list
>>> svn shelf --drop
>>> svn shelf --diff/--show
>>
>> That's a totally reasonable approach, and is similar to (not exactly the
>> same as) what's currently implemented on trunk and on shelve-checkpoint
>> branch.
>>
>> FWIW you missed out 'svn save' (create a checkpoint, don't revert from WC).
>
> That could be 'svn shelve --keep'

Yes it could...

>> The discussion of syntax for referring to a particular shelf and version is
>> perhaps the more interesting part of the whole discussion; I would welcome
>> any thoughts on that part.
>
> We could assign numbers to them, display those in 'svn shelf --list',
> and let all other shelf-manipulating options expect a number.

Sure, that's implemented already:

[[[
$ svn savepoint log 'qq'
version 1: 52 days ago
  0 files changed

version 2: 51 days ago
  hello.txt |    1 +
  1 file changed, 1 insertion(+)

version 3: 32 days ago
  D5/file   |    2 ++
  hello.txt |    1 +
  2 files changed, 3 insertions(+)


$ svn savepoint restore qq 2
U         hello.txt
restored 'qq' version 2 and deleted 1 newer versions

]]]

Now I'm talking about *syntax* for accessing these. I proposed some
syntaxes in the initial email in this thread.

> Additionally, or instead, we could have user-provided shelf entry names
> (UTF-8 strings), and allow them to be used for identification.

I don't like that idea, but thanks for suggesting it anyway; it may
spark some other thoughts.

- Julian

Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Julian Foad-5
Julian Foad wrote:

> [[[
> $ svn savepoint log 'qq'
> version 1: 52 days ago
>   0 files changed
>
> version 2: 51 days ago
>   hello.txt |    1 +
>   1 file changed, 1 insertion(+)
>
> version 3: 32 days ago
>   D5/file   |    2 ++
>   hello.txt |    1 +
>   2 files changed, 3 insertions(+)
>
>
> $ svn savepoint restore qq 2
> U         hello.txt
> restored 'qq' version 2 and deleted 1 newer versions
>
> ]]]
>
> Now I'm talking about *syntax* for accessing these.

Shelved changes are parallel to working changes.

   WC-base
   \ \ \ \__ WC-working
    \ \ \__ shelf 'foo' v1
     \ \__ shelf 'foo' v2
      \__ shelf 'bar' v1

The 'working' state and each 'shelved' state can be considered as a tree
snapshot, and can also be considered as a diff relative to WC-base. In
the near future hopefully we'll be able to track a different base for
each shelf-version; for now they are all (implicitly) considered
relative to the current WC base (even though the patch files already
contain the base revision number of each file; currently we ignore those).

Anyway the main point here is: we have in principle a set of tree
snapshots (no matter that each one is stored in the form of a difference
against the base); and we use revision specifiers to refer to the
working and base versions:

   WC-base                      svn_opt_revision_base     "-r BASE"
   \ \ \ \__ WC-working         svn_opt_revision_working  (no CLI syntax)
    \ \ \__ shelf 'foo' v1
     \ \__ shelf 'foo' v2
      \__ shelf 'bar' v1


  (svn_opt_revision_working, _base, etc.).

We could extend the revision specifiers to refer to shelved versions:

   WC-base                      {svn_opt_revision_base}
   \ \ \ \__ WC-working         {svn_opt_revision_working}
    \ \ \__ shelf 'foo' v1      {svn_opt_revision_shelf,'foo',1}
     \ \__ shelf 'foo' v2       {svn_opt_revision_shelf,'foo',2}
      \__ shelf 'bar' v1        {svn_opt_revision_shelf,'bar',1}

with CLI syntax for example:

   svn cat -r shelf:foo path/to/file
   svn cat path/to/file@shelf:foo
   svn diff -c shelf:foo

The big advantage of some (any) such scheme is that existing
libsvn_client APIs for operating on a working copy change or state (such
as cat, diff, status, prop*, revprop_get, revprop_set, etc.) can then
potentially be extended to support operating on a shelved change or
state without any change to the API function declaration itself.

Just letting you know where my thoughts are going.

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Daniel Shahaf-2
Julian Foad wrote on Tue, 09 Jan 2018 13:31 +0000:
> Anyway the main point here is: we have in principle a set of tree
> snapshots (no matter that each one is stored in the form of a difference
> against the base); and we use revision specifiers to refer to the
> working and base versions:
...
> The big advantage of some (any) such scheme is that existing
> libsvn_client APIs for operating on a working copy change or state (such
> as cat, diff, status, prop*, revprop_get, revprop_set, etc.) can then
> potentially be extended to support operating on a shelved change or
> state without any change to the API function declaration itself.

This resembles the FS API, where svn_fs_root_t can be either a revision
root or a txn root.
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Julian Foad-5
Julian Foad wrote on Tue, 09 Jan 2018 13:31 +0000:
> Anyway the main point here is: we have in principle a set of tree
> snapshots (no matter that each one is stored in the form of a difference
> against the base); and we use revision specifiers to refer to the
> working and base versions:

Initial implementation: http://svn.apache.org/r1820696 "On the
'shelve-checkpoint' branch: Let revision specifiers access shelves."

I think this proves the principle of using revision specifier syntax to
refer to shelves.

$ svn propset -r foo --revprop svn:log 'New log msg.'
$ svn propget -r foo --revprop svn:log
New log msg.

where 'foo' is a shelf. 'propedit' and 'proplist' and 'propdel' work
too. Only the svn:log revprop is stored in the shelf format so far; it
will be easy to extend to support all revprops.

We may well prefer some other syntax than simply "-r foo", such as "-r
shelf:foo" or "-r [foo]" or whatever, but that's not the main point.

The main point is how the rev spec is converted to an (extended)
svn_opt_revision_t object and then passed in to the libsvn_client APIs,
and to what extent we would need to modify the APIs to properly support
this. Already I found, for example, that the revprop APIs take an
svn_opt_revision_t structure as input but they also output a plain
svn_revnum_t to tell what number a date or 'head' revspec was resolved
to; with a shelved change, the output revision specifier cannot be
represented as svn_revnum_t, and I set it to -1 which causes some ugly
output:

$ svn proplist --revprop -r foo
Unversioned properties on revision -1:
   svn:log

So we would want to change this to svn_opt_revision_t. This is just the
first example I have come across of a change needed; some other APIs
will likely require more extensive changes.

Only revprop access is implemented so far, as it was the easiest part to
implement. Providing unified access to a 'tree snapshot' view of the
shelved version (svn list/cat/propget (versioned properties), etc.) and
making modifications (svn add/cp/mv/rm/propset, etc.) and a 'difference'
view (svn diff/status) will be cumbersome to implement, especially with
patch file as the storage, although relatively straightforward in
principle. This could be the trigger for a change in storage form.

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: A rational svn shelve/checkpoint CLI

Julian Foad-5
Julian Foad wrote:
> [...] Only the svn:log revprop is stored in the shelf format so far; it
> will be easy to extend to support all revprops.

http://svn.apache.org/r1820714 does that.

- Julian