It would be nice to be able to look through the patches in ones own repo in the
same way that one can look through patches in "darcs pull". unpull sorta helps
with that, but it doesn't feel warm and fuzzy, and it doesn't help much with
depended-on patches...

Perhaps this should be "darcs changes --interactive"? That would seem more
consistent to me.
-- 
David Roundy

Yeah, probably ;-)

> It would be nice to be able to look through the patches in ones own repo in the
> same way that one can look through patches in "darcs pull". unpull sorta helps
> with that, but it doesn't feel warm and fuzzy, and it doesn't help much with
> depended-on patches...

Attached is my attempt at implementing this as an --interactive option
to changes, as suggested by David.  I went into some unknown
territories here so if anyone has time to sanity check this I would
appreciate it.

Specifically, I'm curious if "s <- slurp_recorded repository" in
Changes.lhs is the correct slurpy to pass in.  The value isn't
actually used (with_any_selected_changes ignores the slurpy parameter)
but it probably should contain the correct value either way.

Attachments

• support_interactive_option_in_changes_command_issue_59.dpatch

I notice that this patch won't apply to my partial repo...

Okay, I've been directed to the darcs-unstable repo and after pulling from that
this patch applies nicely. It works nicely too ;-). Thank you, kapheine1! How
can this get into the darcs-unstable repo?

> Attached is my attempt at implementing this as an --interactive option
> to changes,

It looks very useful to me, so I'm applyting it notwithstanding your doubts.

> Specifically, I'm curious if "s <- slurp_recorded repository" in
> Changes.lhs is the correct slurpy to pass in.  The value isn't
> actually used (with_any_selected_changes ignores the slurpy parameter)
> but it probably should contain the correct value either way.

David, Ian, could either of your clarify that?  My gut instinct is
that we should remove the extra argument unless someone can explain
what purpose it was meant to serve.

                                        Juliusz

Zachary,

I'm already finding this useful -- but there's one thing I find
confusing.  The prompts says ``Shall I continue to view changes?'',
but answering ``y'' or ``n'' has no effect.

                                        Juliusz

Oh, and a feature request -- any chance for a key that allows viewing
in the format of ``darcs send -u'' ?

                                        Juliusz

On Fri, Jan 06, 2006 at 04:57:29PM +0100, Juliusz Chroboczek wrote:
> 
> > Specifically, I'm curious if "s <- slurp_recorded repository" in
> > Changes.lhs is the correct slurpy to pass in.  The value isn't
> > actually used (with_any_selected_changes ignores the slurpy parameter)
> > but it probably should contain the correct value either way.
> 
> David, Ian, could either of your clarify that?  My gut instinct is
> that we should remove the extra argument unless someone can explain
> what purpose it was meant to serve.

I think it's on its way to allowing you to see context of patches while
you select.

(Ultimately it probably wants to be something that can be either a
slurpy in memory or a temporary copy on disk).

Thanks
Ian

>>> Specifically, I'm curious if "s <- slurp_recorded repository" in
>>> Changes.lhs is the correct slurpy to pass in.  The value isn't
>>> actually used (with_any_selected_changes ignores the slurpy parameter)

> I think it's on its way to allowing you to see context of patches while
> you select.

Ah, okay.  So it looks like the value currently being passed is the
right one.

Thanks for your help.

                                        Juliusz

On 1/6/06, Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> wrote:
> Zachary,
>
> I'm already finding this useful -- but there's one thing I find
> confusing.  The prompts says ``Shall I continue to view changes?'',
> but answering ``y'' or ``n'' has no effect.

Juliusz,

Heh, yes.  So it does.  I'm not quite sure how this should work. We
could have "y" be the same as "k" and "n" be the same as "q".  But I'm
not sure it is a good idea to have multiple letters do the same thing.
 So another option might be to have the prompt say something like
"Next command?" instead.  Does anyone have any thoughts?

On 1/6/06, Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> wrote:
> Oh, and a feature request -- any chance for a key that allows viewing
> in the format of ``darcs send -u'' ?

Juliusz,

I wrote up a little patch to do this, but I think that if we use it,
we will also need to add it to all of the interactive patch selecting
commands, for consistency.  Is this a good idea?  Personally I don't
mind the normal (non-u) format for this type of patch viewing, but if
other people use it I see no reason why we shouldn't add it.

If people do want the future, what should the keys be?  We'll need one
for showing the -u style and another for paging -u (parallels of 'v'
and 'p')

On Fri, Jan 06, 2006 at 07:47:09PM +0000, Ian Lynagh wrote:
> On Fri, Jan 06, 2006 at 04:57:29PM +0100, Juliusz Chroboczek wrote:
> > 
> > > Specifically, I'm curious if "s <- slurp_recorded repository" in
> > > Changes.lhs is the correct slurpy to pass in.  The value isn't
> > > actually used (with_any_selected_changes ignores the slurpy
> > > parameter) but it probably should contain the correct value either
> > > way.
> > 
> > David, Ian, could either of your clarify that?  My gut instinct is that
> > we should remove the extra argument unless someone can explain what
> > purpose it was meant to serve.
> 
> I think it's on its way to allowing you to see context of patches while
> you select.

Right, that's the idea, it just never got implemented.  It shouldn't be too
hard, it's just that we'd need to bit of bookkeeping to update the slurpy
for each patch we want to view with context.
-- 
David Roundy
http://www.darcs.net

On Sat, Jan 07, 2006 at 02:26:37AM +0000, Zachary P. Landau wrote:
> If people do want the future, what should the keys be?  We'll need one
> for showing the -u style and another for paging -u (parallels of 'v'
> and 'p')

A radical thought would be to drop the -u flags and use the
-u format always everywhere possible.  I don't think it will
be hard for anyone getting used to.  If darcs had stared out
this way, would anyone ever have thought of wishing for the
"cramped" format?

> Heh, yes.  So it does.  I'm not quite sure how this should work. We
> could have "y" be the same as "k" and "n" be the same as "q".  But I'm
> not sure it is a good idea to have multiple letters do the same thing.
>  So another option might be to have the prompt say something like
> "Next command?" instead.  Does anyone have any thoughts?

I'm hopeless at UI design, but I like the former better -- it makes it
pretty clear that pressing ``y'' will Do The Right Thing.

I also suggest that just pressing enter is equivalent to pressing n.

                                        Juliusz

> I wrote up a little patch to do this,

I would definitely use the feature.

> but I think that if we use it, we will also need to add it to all of
> the interactive patch selecting commands, for consistency.  Is this
> a good idea?

I believe it is, but as far as I'm concerned, it can wait.  (I tend to
``pull -a'', then review the changes, and unpull anything I didn't
like.)

> If people do want the future, what should the keys be?  We'll need one
> for showing the -u style and another for paging -u (parallels of 'v'
> and 'p')

Upper-case `V' and `P'?  Or does anyone have a better idea?

                                        Juliusz

On 1/9/06, Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> wrote:
> > Heh, yes.  So it does.  I'm not quite sure how this should work. We
> > could have "y" be the same as "k" and "n" be the same as "q".  But I'm
> > not sure it is a good idea to have multiple letters do the same thing.
> >  So another option might be to have the prompt say something like
> > "Next command?" instead.  Does anyone have any thoughts?
>
> I'm hopeless at UI design, but I like the former better -- it makes it
> pretty clear that pressing ``y'' will Do The Right Thing.
>

I don't have a problem with this.

> I also suggest that just pressing enter is equivalent to pressing n.

I am a little bit worried about this.  Enter becomes a Quit key, which
seems awkward.  But maybe that is not such a bad thing since it is
only for patch reviewing, not for any real action.  I have no strong
opinion, really.

On 1/9/06, Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> wrote:
> > If people do want the future, what should the keys be?  We'll need one
> > for showing the -u style and another for paging -u (parallels of 'v'
> > and 'p')
>
> Upper-case `V' and `P'?  Or does anyone have a better idea?

I believe capital letters are purposefully unused because the commands
are shown capitalized in the prompt to show which one is the default.

Maybe 'u' and 'i'?  u for unified and i because it is next to u :P

>> I'm hopeless at UI design, but I like the former better -- it makes it
>> pretty clear that pressing ``y'' will Do The Right Thing.

[...]

>> I also suggest that just pressing enter is equivalent to pressing n.

I meant ``y'', of course.

> I am a little bit worried about this.

Sorry for the confustion.

                                        Juliusz

> Maybe 'u' and 'i'?  u for unified and i because it is next to u

No objection as far as I'm concerned, although I'm still hoping
someone will have a better idea than ``i''.

                                        Juliusz

On 1/9/06, Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> wrote:
> > Heh, yes.  So it does.  I'm not quite sure how this should work. We
> > could have "y" be the same as "k" and "n" be the same as "q".  But I'm
> > not sure it is a good idea to have multiple letters do the same thing.
> >  So another option might be to have the prompt say something like
> > "Next command?" instead.  Does anyone have any thoughts?
>
> I'm hopeless at UI design, but I like the former better -- it makes it
> pretty clear that pressing ``y'' will Do The Right Thing.
>
> I also suggest that just pressing enter is equivalent to pressing n.

Juliusz,

Attached is a patch implementing with your proposed changes.

Attachments

• add_y_and_n_commands_to_changes_interactive.dpatch

following patch included in 1.0.6
Wed Dec 21 06:20:49 CET 2005  Zachary P. Landau <kapheine@divineinvasion.net>
  * Support --interactive option in changes command (issue #59).

What about the y/n-think, it's too soon to close this wish yet, or?

> following patch included in 1.0.6
> Wed Dec 21 06:20:49 CET 2005  Zachary P. Landau <kapheine@divineinvasion.net>
>   * Support --interactive option in changes command (issue #59).
>
> What about the y/n-think, it's too soon to close this wish yet, or?

I don't know if you saw it, but I did post a followup patch to
implement y/n.  Whether or not it should be included, I'm not sure.  I
understand the case for consistency, but it also may be a little
confusing because you aren't being asked a question :P

I just got a first impression of trying this command, without having followed     
any of the previous design discussions from this ticket. I was interested to     
see that my ideas had already been discussed. Here are the points that     
immediately struck me:      
     
- I am being asked a "yes/no" question, so I expected "y" and "n" to work. As     
someone else mentioned "y" should map to "j" to proceed, and "n"  to "q" to    
quit.      
     
- I expected Enter to also map to "j" to proceed, as this approximates what     
more and less do, and paging is more or less what we are doing. :)  Instead,     
something scary happened. darcs hung. I quickly backgrounded it and checked to     
see if it was pegging my CPU. It wasn't. I soon realized it was just sitting     
there, waiting for more input. If "Enter" doesn't get mapped to "yes, proceed",     
that it should at least cause the prompt to be repeated, which makes it very     
clear it is still waiting for input.      
     
Thanks for this command, I like it!

On 06/01/06, Zachary P. Landau <bugs@darcs.net> wrote:
>
> Zachary P. Landau <kapheine@gmail.com> added the comment:
>
> On 1/6/06, Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> wrote:
> > Zachary,
> >
> > I'm already finding this useful -- but there's one thing I find
> > confusing.  The prompts says ``Shall I continue to view changes?'',
> > but answering ``y'' or ``n'' has no effect.
>
> Juliusz,
>
> Heh, yes.  So it does.  I'm not quite sure how this should work. We
> could have "y" be the same as "k" and "n" be the same as "q".  But I'm
> not sure it is a good idea to have multiple letters do the same thing.
>  So another option might be to have the prompt say something like
> "Next command?" instead.  Does anyone have any thoughts?

"What now?"