Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

2018-01-18 Thread Jacob Keller 
On Thu, Jan 18, 2018 at 2:08 PM, Philip Oakley  wrote:
> From: "Jacob Keller" 
>>
>> On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller 
>> wrote:
>>>
>>> Jake suggested using "x false" instead of "edit" for some corner cases.
>>>
>>> I do prefer using "x false" for all kinds of things such as stopping
>>> before a commit (edit only let's you stop after a commit), and the
>>> knowledge that "x false" does the least amount of actions behind my back.
>>>
>>> We should have that command as well, maybe?
>>>
>>
>>
>> I agree. I use "x false" very often, and I think stop is probably a
>> better solution since it avoids spawning an extra shell that will just
>> fail. Not sure if stop implies too much about "stop the whole thing"
>> as opposed to "stop here and let me do something manual", but I think
>> it's clear enough.
>>
> 'hold' or 'pause' maybe options (leads to
> http://www.thesaurus.com/browse/put+on+hold offering procastinate etc.)
> 'adjourn'.
>
>

I like break, as suggested by Dscho. That also works well for
abbreviation if we drop the "bud" command.

Thanks,
Jake

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

2018-01-18 Thread Stefan Beller 
On Thu, Jan 18, 2018 at 2:00 PM, Johannes Schindelin
 wrote:

>> + TODO_STOP,
>
> I see that your original idea was "stop", but then you probably realized
> that there would be no good abbreviation for that, and changed your mind.
>
> Personally, I would have called it `break`...

I was looking at a synonym list of stop to find a word that contained a letter
which was not already taken. 'break' would allow for 'a', or 'k', assuming 'bud'
takes 'b' (or can that go to 'u'? Are there people out there with muscle memory
on these letters already?)

Any word (of stop, break, stay, control) sounds good to me, though 'break' might
be the clearest.

>
>> @@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
>> struct replay_opts *opts)
>>   /* `current` will be incremented below */
>>   todo_list->current = -1;
>>   }
>> + } else if (item->command == TODO_STOP) {
>> + todo_list->current = -1;
>
> That is incorrect, it will most likely write an unexpected `done` file.
>
> Did you mean `return 0` instead?

I guess. I did not compile or test the patch, I was merely writing down enough
to convey the idea, hopefully.

While talking about this idea of exploding the number of keywords,
maybe we can also have 'abort', which does the same as deleting all lines
(every time I want to abort I still get shivers if I just drop all
patches instead
of aborting, so maybe typing 'abort-and-restore' as the first thing in the file
would convey a safer feeling to users?)

Thanks for taking these additional considerations into mind while I don't
review the actual patches,

Stefan

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

2018-01-18 Thread Philip Oakley 

From: "Jacob Keller" 
On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller  
wrote:

Jake suggested using "x false" instead of "edit" for some corner cases.

I do prefer using "x false" for all kinds of things such as stopping
before a commit (edit only let's you stop after a commit), and the
knowledge that "x false" does the least amount of actions behind my back.

We should have that command as well, maybe?




I agree. I use "x false" very often, and I think stop is probably a
better solution since it avoids spawning an extra shell that will just
fail. Not sure if stop implies too much about "stop the whole thing"
as opposed to "stop here and let me do something manual", but I think
it's clear enough.

'hold' or 'pause' maybe options (leads to 
http://www.thesaurus.com/browse/put+on+hold offering procastinate etc.)

'adjourn'.




Signed-off-by: Stefan Beller 
---
 git-rebase--interactive.sh |  1 +
 sequencer.c| 10 ++
 2 files changed, 11 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3cd7446d0b..9eac53f0c5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -166,6 +166,7 @@ l, label = label current HEAD with a name
 t, reset  = reset HEAD to a label
 b, bud = reset HEAD to the revision labeled 'onto', no arguments
 m, merge []* = create a merge commit using a given 
commit's message

+y, stay = stop for  shortcut for

 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 2b4e6b1232..4b3b9fe59d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -782,6 +782,7 @@ enum todo_command {
TODO_RESET,
TODO_BUD,
TODO_MERGE,
+   TODO_STOP,
/* commands that do nothing but are counted for reporting 
progress */

TODO_NOOP,
TODO_DROP,
@@ -803,6 +804,7 @@ static struct {
{ 'l', "label" },
{ 't', "reset" },
{ 'b', "bud" },
+   { 'y', "stay" },
{ 'm', "merge" },
{ 0,   "noop" },
{ 'd', "drop" },
@@ -1307,6 +1309,12 @@ static int parse_insn_line(struct todo_item *item, 
const char *bol, char *eol)

return 0;
}

+   if (item->command == TODO_STOP) {
+   item->commit = NULL;
+   item->arg = "";
+   item->arg_len = 0;
+   }
+
end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
item->arg = end_of_object_name + strspn(end_of_object_name, " 
\t");

item->arg_len = (int)(eol - item->arg);
@@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list 
*todo_list, struct replay_opts *opts)

/* `current` will be incremented below */
todo_list->current = -1;
}
+   } else if (item->command == TODO_STOP) {
+   todo_list->current = -1;
} else if (item->command == TODO_LABEL)
res = do_label(item->arg, item->arg_len);
else if (item->command == TODO_RESET)
--
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

2018-01-18 Thread Johannes Schindelin 
Hi Stefan,

On Thu, 18 Jan 2018, Stefan Beller wrote:

> Jake suggested using "x false" instead of "edit" for some corner cases.
> 
> I do prefer using "x false" for all kinds of things such as stopping
> before a commit (edit only let's you stop after a commit), and the
> knowledge that "x false" does the least amount of actions behind my back.
> 
> We should have that command as well, maybe?
> 
> Signed-off-by: Stefan Beller 
> ---
>  git-rebase--interactive.sh |  1 +
>  sequencer.c| 10 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3cd7446d0b..9eac53f0c5 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -166,6 +166,7 @@ l, label = label current HEAD with a name
>  t, reset  = reset HEAD to a label
>  b, bud = reset HEAD to the revision labeled 'onto', no arguments
>  m, merge []* = create a merge commit using a given commit's 
> message
> +y, stay = stop for  shortcut for
>  
>  These lines can be re-ordered; they are executed from top to bottom.
>  " | git stripspace --comment-lines >>"$todo"
> diff --git a/sequencer.c b/sequencer.c
> index 2b4e6b1232..4b3b9fe59d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -782,6 +782,7 @@ enum todo_command {
>   TODO_RESET,
>   TODO_BUD,
>   TODO_MERGE,
> + TODO_STOP,

I see that your original idea was "stop", but then you probably realized
that there would be no good abbreviation for that, and changed your mind.

Personally, I would have called it `break`...

> @@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
> struct replay_opts *opts)
>   /* `current` will be incremented below */
>   todo_list->current = -1;
>   }
> + } else if (item->command == TODO_STOP) {
> + todo_list->current = -1;

That is incorrect, it will most likely write an unexpected `done` file.

Did you mean `return 0` instead?

Ciao,
Dscho

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

2018-01-18 Thread Jacob Keller 
On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller  wrote:
> Jake suggested using "x false" instead of "edit" for some corner cases.
>
> I do prefer using "x false" for all kinds of things such as stopping
> before a commit (edit only let's you stop after a commit), and the
> knowledge that "x false" does the least amount of actions behind my back.
>
> We should have that command as well, maybe?
>


I agree. I use "x false" very often, and I think stop is probably a
better solution since it avoids spawning an extra shell that will just
fail. Not sure if stop implies too much about "stop the whole thing"
as opposed to "stop here and let me do something manual", but I think
it's clear enough.

Thanks,
Jake

> Signed-off-by: Stefan Beller 
> ---
>  git-rebase--interactive.sh |  1 +
>  sequencer.c| 10 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3cd7446d0b..9eac53f0c5 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -166,6 +166,7 @@ l, label = label current HEAD with a name
>  t, reset  = reset HEAD to a label
>  b, bud = reset HEAD to the revision labeled 'onto', no arguments
>  m, merge []* = create a merge commit using a given commit's 
> message
> +y, stay = stop for  shortcut for
>
>  These lines can be re-ordered; they are executed from top to bottom.
>  " | git stripspace --comment-lines >>"$todo"
> diff --git a/sequencer.c b/sequencer.c
> index 2b4e6b1232..4b3b9fe59d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -782,6 +782,7 @@ enum todo_command {
> TODO_RESET,
> TODO_BUD,
> TODO_MERGE,
> +   TODO_STOP,
> /* commands that do nothing but are counted for reporting progress */
> TODO_NOOP,
> TODO_DROP,
> @@ -803,6 +804,7 @@ static struct {
> { 'l', "label" },
> { 't', "reset" },
> { 'b', "bud" },
> +   { 'y', "stay" },
> { 'm', "merge" },
> { 0,   "noop" },
> { 'd', "drop" },
> @@ -1307,6 +1309,12 @@ static int parse_insn_line(struct todo_item *item, 
> const char *bol, char *eol)
> return 0;
> }
>
> +   if (item->command == TODO_STOP) {
> +   item->commit = NULL;
> +   item->arg = "";
> +   item->arg_len = 0;
> +   }
> +
> end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
> item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
> item->arg_len = (int)(eol - item->arg);
> @@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
> struct replay_opts *opts)
> /* `current` will be incremented below */
> todo_list->current = -1;
> }
> +   } else if (item->command == TODO_STOP) {
> +   todo_list->current = -1;
> } else if (item->command == TODO_LABEL)
> res = do_label(item->arg, item->arg_len);
> else if (item->command == TODO_RESET)
> --
> 2.16.0.rc1.238.g530d649a79-goog
>

[PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

2018-01-18 Thread Stefan Beller 
Jake suggested using "x false" instead of "edit" for some corner cases.

I do prefer using "x false" for all kinds of things such as stopping
before a commit (edit only let's you stop after a commit), and the
knowledge that "x false" does the least amount of actions behind my back.

We should have that command as well, maybe?

Signed-off-by: Stefan Beller 
---
 git-rebase--interactive.sh |  1 +
 sequencer.c| 10 ++
 2 files changed, 11 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3cd7446d0b..9eac53f0c5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -166,6 +166,7 @@ l, label = label current HEAD with a name
 t, reset  = reset HEAD to a label
 b, bud = reset HEAD to the revision labeled 'onto', no arguments
 m, merge []* = create a merge commit using a given commit's 
message
+y, stay = stop for  shortcut for
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 2b4e6b1232..4b3b9fe59d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -782,6 +782,7 @@ enum todo_command {
TODO_RESET,
TODO_BUD,
TODO_MERGE,
+   TODO_STOP,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -803,6 +804,7 @@ static struct {
{ 'l', "label" },
{ 't', "reset" },
{ 'b', "bud" },
+   { 'y', "stay" },
{ 'm', "merge" },
{ 0,   "noop" },
{ 'd', "drop" },
@@ -1307,6 +1309,12 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
return 0;
}
 
+   if (item->command == TODO_STOP) {
+   item->commit = NULL;
+   item->arg = "";
+   item->arg_len = 0;
+   }
+
end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
item->arg_len = (int)(eol - item->arg);
@@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
/* `current` will be incremented below */
todo_list->current = -1;
}
+   } else if (item->command == TODO_STOP) {
+   todo_list->current = -1;
} else if (item->command == TODO_LABEL)
res = do_label(item->arg, item->arg_len);
else if (item->command == TODO_RESET)
-- 
2.16.0.rc1.238.g530d649a79-goog