[darcs-users] short options and long names (VOTE!)

[David Roundy]

On Tue, Jul 27, 2004 at 03:11:50AM -0400, Andrew Pimlott wrote:
> [Replying to old mail.]
> 
> On Fri, Jun 11, 2004 at 06:37:28AM -0400, David Roundy wrote:
> > I've implemented this suggestion, since it seemed good to me.  Flags now
> > specify either a single patch (--patch), several patches (--patches) or
> > one end of a patch range (--from-patch and --to-patch).
> 
> I started trying to update the documentation for this, but I found that
> I still don't understand these options.  Let me go through the commands
> that use them and explain my confusion.

Thanks for the feedback!

> As preface, I think some of it stems from the fact that a range of
> patches is an ill-defined concept, since patches are not fully ordered,
> even though it is practically useful (at least today).  I think darcs
> should make a decision about where this concept is useful and supported,
> and deprecate its use outside of those places.  Similarly, --patch isn't
> well-defined, since it takes the "last" matching patch.

I think I disagree here.  Operationally, the "order" of patches is a
well-defined property, even though it is mutable and not really important.
And in gives a very useful way to select patches.  "changes" shows the
order, so it needn't give a surprise.

The "last" matching patch is well-defined, and is useful, since it means
you can easily run a changes, see the patch you want, see a word in it that
doesn't show up in any more recent patches and run with --patch "word" and
know that it's what you want.

> - get takes --patch.  However, this really specifies a range that
>   implicitly starts at the beginning.  Maybe this should be made
>   explicit with --to-patch.

Agreed.  I've changed this to --to-patch and --to-match.

> - pull, push, and send take --patches (implying these patches and their
>   dependents).  The discrepancy with get jumps out at me: if it makes
>   sense to get a range with get, why not with pull?  Or ask the question
>   the other way around.

No, the plural here doesn't indicate the dependencies, but rather the fact
that these commands don't operate on the "last" matching patch, but rather
on *all* matching patches.  This is an important distinction, especially if
one were to thrown in the --all flag, which would give you all matching
patches.

> - apply takes nothing.  Might you ever want to pick from a sent
>   patch bundle?

You might, but I don't really see a need to do this any way other than
interactively...

> - rollback, unrecord, and unpull take --patch.  Is there any particular
>   reason not to allow multiple patches here?  In fact, I would argue
>   that the current semantic is closer to --patches, since you are
>   prompted with a series of patches until you say 'y'.

Each of these commands only operates on a single patch.  They *could* be
made to operate on multiple patches, but it would be more trouble, and they
are sufficiently dangerous that I don't really see that they should be done
"en masse".

> - rerecord takes --patch.  I haven't used this much, but this seems
>   right, except that using --tag here would be bizarre.  (Hmm... you
>   might want to combine multiple patches into one, in which case you
>   might want --patches to specify the patches, and --patch-name to name
>   the new patch.)

You're right, --tag would be bizarre and I've now eliminated it.  Using
rerecord with multiple patches doesn't seem worthwhile, since rerecord
doesn't change the patch name/comment, so you'd need a more complicated
interface (as you say) to decide which name and comment to keep.

> - changes takes --from-patch, but not --to-patch.  What's up with that?

That's just because it's not implemented.  I've never really wanted to see
just "old" changes.  Seeing the changes since the last tag (e.g. 0.9.22) is
something I often want, since it's how I come up with the release
announcement for a new darcs.

>   A range is probably ok for an informational command like changes.

Definitely... that's precisely what changes outputs.

>   However you probably often want something different: the set of
>   changes in tag a, but not tag b.  AFAICT there's no way to get that.

Yeah, that would be a nice thing to know.  Needs an interface, and a bit of
small work, but not a hard thing to come up with.  The interface is the
tough bit.  I guess maybe --from-tag and --to-tag, but then I'd have to
deal with how to display changes that are *in* the --from-tag and missing
from the --to-tag.  Yuck.  I guess I could just invert them so they show up
as "UNDO" changes.

>   Also, you might want to see the changes from a single patch (more
>   plausible with diff, but even with changes you can see the files
>   affected), so maybe this command should also take --patch.  Or should
>   it be --patches?

This is what is output by annotate.  This is a continual source of
confusion, since noone expects annotate to do that.  I *could* move this
functionality into changes, but perhaps had better add a separate "show" or
"showpatch" command to do this.  But I sort of feel it belongs in annotate
(see below).

>   BTW, --from-patch does not include the specified patch, which I think
>   is usually counterintuitive, and prevents you from including the first
>   patch.  I think --from and --to should both be inclusive (probably
>   even if they define a reverse range).

Hmmm.  I'd rather have it non-inclusive (or at least, that's what I decided
when implementing it).  It's a bit hard to say which is better.  I guess
maybe inclusive is better, since it's easy to ignore a change you didn't
want to see, but hard to see a change that isn't displayed...

> - diff takes --from-patch and --to-patch.  Similar to changes.

This is awkward.  Diff really *should* take a --patch option as well,
except that if --patch was specified, you'd have an error if you specified
either --from-patch or --to-patch.

It's also awkward that I never actually use diff, so my opinion on the
matter isn't really well-formed (or trustworthy).

> - annotate takes --patch.  This has totally different meanings in the
>   two behaviors of annotate, which is way confusing.  In patch output
>   mode, it should probably be similar to changes.  In annotate mode,
>   --patch is probably ok, although maybe it should be --to-patch.

Here I'll explain how I think about annotate.

Annotate *always* describes a single patch.  By default it is the most
recent patch.  If a file is specified, annotate describes the changes to
that file (along with the history of each line of the file).  If a
directory is given, it shows the changes to that directory (along with the
a bit of history of each file in the directory).  With no file or directory
given, it outputs all the changes of that patch in darcs patch format.

Looked at in this way, it makes (perhaps) some sense.  On the other hand,
since it gives (when asking about files or directories) the entire context
of the change, it also can be used to look at "historical" versions of a
file.  Except that perhaps those "historical" versions never
existed... which is why I don't think of that as being its purpose.

Annotate also gives a history of which patch added or removed each line of
a file, which is also very helpful (and better guaranteed to be "valid"
than the actual contents of the "historical" version of the file).

Anyhow, that's the "reasoning" such as it is, for why annotate does what it
does.  changes was intended originally just to output something like a
changelog, and has morphed to output more (via the --summary option).  I
see that the help description of annotate is incredibly sparse...

Anyhow, suggestions on this front of the interface are still welcome, but
I'll have to be convinced that we have a unified picture that would make
sense to newbies before I'm likely to make any changes.

> My thoughts aren't entirely coherent right now, but here is a proposal
> to think about:

(Some of these will get double-responded to, but such is life...)

> - Eliminate the singular/plural distinction and just use --patch.  This
>   is always plural; in diff and rerecord, it is an error if this
>   designates multiple patches (or maybe they should just be exceptions,
>   and take the latest match).  (BTW, --tag should probably also be
>   plural for consistency, even though the distinction will almost never
>   arise.)  Maybe make --patch and --patches synonyms, for when one or
>   the other is more natural, but emphasize that they are the same.

As I said above, I believe that --patch and --patches are very different
options.  One selects the single latest matching patch, and the other
selects all matching patches.  I think this is both a useful way of doing
the interface, and an important distinction (as in wanting to avoid the
"surprise" factor).

> - Remove all [pm]atch options from get.  You can only get tags.  I
>   assume (didn't test) that --tag is interpreted as a range, just like
>   (The two behaviors are usually indistinguishable, but I think if tags
>   were created in separate repositories, they might not be.)

Instead I changed then to --to-[pm]atch.  --tag is not interpereted as a
range, but rather as a "tag".  That is, a specific version.  And you're
right, treating it as a range is distinguishable from treating it as a
"version".

> - Add --patch to apply.  Why not?

I'm lazy.  :) No reason not to, except that it's perhaps a bit silly.
Probably all of two lines of code to add this.

> - rollback, unrecord, and unpull all keep going after the user presses
>   'y'.  Add a 'd'one command to stop.

I don't like this.  I prefer them just operating on a single patch.  It's a
pain if you want to unpull a whole bunch of patches, but such is life.  I
don't really want it to be easy to destroy your repository.  And for
experienced users, you can get similar effects with get --to-patch.

> - Eliminate --tag from rerecord.

Done.

> - changes and diff both take --from-patch, --to-patch, and --patch.

Sounds good, but not done.  Also there's still the question of inclusive
vs. exclusive --from-patch.  Right now it's always exclusive.

> - Put the patch output mode of annotate into changes.

See above.

> - Use --to-patch for annotate mode of annotate.

Again see above, but basically it depends what you see annotate as
showing.  If it's showing the changes of a single patch in context, it's
already correct.  If you view it as showing the result of all previous
patches, then you're correct.  I prefer the former view, but probably
either the docs should make that clear, or the interface should change, or
something else perhaps needs to happen.

> BTW, I like the --last option that was recently added.  However, I don't
> think it should fail if more than the number of patches in the repo is
> specified; it should just print what it has.

Hmmm.  I've fixed this.

> PS.  I will answer email faster this time.  I was moving last month.

No problem! (Although it did have the result that these changes won't make
it into darcs 0.9.23--there's too high a risk that darcs.cgi will get
messed up.)
-- 
David Roundy
http://www.abridgegame.org