Re: [PATCH] rebase -i: Teach "--edit" action

6 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] rebase -i: Teach "--edit" action

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:54:42

Andrew Wong [off-list ref] writes:
This allows users to edit the todo list while they're in the middle of
an interactive rebase.
I like the idea.
+edit)
+  git_sequence_editor "$todo" ||
+    die_abort "Could not execute editor"
+
+  exit
+  ;;
Indent with space. Please, use tabs (same below).
quoted hunk
index 15da926..c394b8d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -38,6 +38,7 @@ C=!                passed to 'git apply'
 continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
+edit!              edit the todo list during interactive rebase
Just "edit" may be a bit misleading, as we already have the "edit"
action inside the todolist. I'd call this --edit-list to avoid
ambiguity.

This lacks tests, IMHO, as there are many corner-cases (e.g. should we
be allowed to --edit-list while the worktree is in conflict?) that would
deserve to be at least discussed, and as much as possible automatically
tested.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: Teach "--edit" action

From: Andrew Wong <hidden>
Date: 2016-06-15 22:54:42

On Mon, Sep 10, 2012 at 12:25 PM, Matthieu Moy
[off-list ref] wrote:
Indent with space. Please, use tabs (same below).
Ah, thanks. Good catch.
Just "edit" may be a bit misleading, as we already have the "edit"
action inside the todolist. I'd call this --edit-list to avoid
ambiguity.
I thought that might be a bit confusing too. "--edit-list" doesn't
seem informative about what "list" we're editing though. What about
"--edit-todo"? Any suggestions are welcomed.
This lacks tests, IMHO, as there are many corner-cases (e.g. should we
be allowed to --edit-list while the worktree is in conflict?) that would
deserve to be at least discussed, and as much as possible automatically
tested.
It does seem risky to do, since we're exposing something that used to
be internal to "rebase -i". Though I don't see harm in allowing
modifications even when there's a conflict, since we're not really
committing anything, modifying index, or any worktree file. As long as
the todo file exists, and we're stopped in the middle of a rebase, I
think editing it shouldn't cause any problems.

Re: [PATCH] rebase -i: Teach "--edit" action

From: Jeff King <hidden>
Date: 2016-06-15 22:54:42

On Mon, Sep 10, 2012 at 12:46:45PM -0400, Andrew Wong wrote:
quoted
Just "edit" may be a bit misleading, as we already have the "edit"
action inside the todolist. I'd call this --edit-list to avoid
ambiguity.
I thought that might be a bit confusing too. "--edit-list" doesn't
seem informative about what "list" we're editing though. What about
"--edit-todo"? Any suggestions are welcomed.
Does it ever make sense to edit and then _not_ immediately continue?
You can't affect the current commit anyway (it has already been pulled
from the todo list), so the next thing you'd want to do it actually act
on whatever you put into the todo list[1].

What if it was called --continue-with-edit or something, and then:
quoted
This lacks tests, IMHO, as there are many corner-cases (e.g. should we
be allowed to --edit-list while the worktree is in conflict?) that would
deserve to be at least discussed, and as much as possible automatically
tested.
We would not even allow the edit if we were not OK to continue.

-Peff

[1] It does preclude using "--edit" to make a note about a later commit
    while you are in the middle of resolving a conflict or something.
    You'd have to do it at the end. I don't know if anybody actually
    cares about that.

Re: [PATCH] rebase -i: Teach "--edit" action

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:54:42

Am 10.09.2012 18:54, schrieb Jeff King:
On Mon, Sep 10, 2012 at 12:46:45PM -0400, Andrew Wong wrote:
quoted
quoted
Just "edit" may be a bit misleading, as we already have the "edit"
action inside the todolist. I'd call this --edit-list to avoid
ambiguity.
I thought that might be a bit confusing too. "--edit-list" doesn't
seem informative about what "list" we're editing though. What about
"--edit-todo"? Any suggestions are welcomed.
Does it ever make sense to edit and then _not_ immediately continue?
Yes. For example, while you are resolving a conflict, you might notice
that it would make sense to do something different in the remaining
rebase sequence. You don't want to continue if some conflicts remain.
And you don't want to wait editing the todo list until you are done with
the conflicts because you might have forgotten that you wanted to do
something different.
You can't affect the current commit anyway (it has already been pulled
from the todo list), so the next thing you'd want to do it actually act
on whatever you put into the todo list[1].
Oh, you said it here:
[1] It does preclude using "--edit" to make a note about a later commit
    while you are in the middle of resolving a conflict or something.
    You'd have to do it at the end. I don't know if anybody actually
    cares about that.
Yes, I do care. At times I tend to have a very short attention span. Or
it is Windows's slowness that expires my short-term memory more often
than not. ;)

-- Hannes

Re: [PATCH] rebase -i: Teach "--edit" action

From: Jeff King <hidden>
Date: 2016-06-15 22:54:42

On Mon, Sep 10, 2012 at 08:36:43PM +0200, Johannes Sixt wrote:
quoted
[1] It does preclude using "--edit" to make a note about a later commit
    while you are in the middle of resolving a conflict or something.
    You'd have to do it at the end. I don't know if anybody actually
    cares about that.
Yes, I do care. At times I tend to have a very short attention span. Or
it is Windows's slowness that expires my short-term memory more often
than not. ;)
OK, then I withdraw my proposal. :)

It sounds like it would be safe to do:

  git rebase --edit-todo
  hack hack hack
  git rebase --continue

anyway, so the restriction is not as valuable as it would otherwise have
been.

-Peff

Re: [PATCH] rebase -i: Teach "--edit" action

From: Andrew Wong <hidden>
Date: 2016-06-15 22:54:42

On Mon, Sep 10, 2012 at 2:46 PM, Jeff King [off-list ref] wrote:
On Mon, Sep 10, 2012 at 08:36:43PM +0200, Johannes Sixt wrote:
quoted
quoted
[1] It does preclude using "--edit" to make a note about a later commit
    while you are in the middle of resolving a conflict or something.
    You'd have to do it at the end. I don't know if anybody actually
    cares about that.
Yes, I do care. At times I tend to have a very short attention span. Or
it is Windows's slowness that expires my short-term memory more often
than not. ;)
OK, then I withdraw my proposal. :)

It sounds like it would be safe to do:

  git rebase --edit-todo
  hack hack hack
  git rebase --continue
Johannes took the words right out of my mouth.  Also, "edit and _not_
continue" also gives the user a chance to second guess while editing
the todo.

That got me thinking... Currently, the todo list has this line at the bottome:
    # However, if you remove everything, the rebase will be aborted.

We'd probably want to remove that line, since "remove everything" no
longer aborts the rebase. It'll just finish the rebase.  It'll be ugly
to sed it out.  Maybe one way to do this is to remove all the comments
and append new ones.

It might also be nice to add a note to remind the user that they're
editing a todo file in a stopped rebase state. i.e. not a fresh
interactive rebase
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help