Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command
- From
Stefan Beller <sbeller@google.com>
- Date
- Jan 18, 2018, 22:09 UTC
- Message-ID
- <CAGZ79kYn5cuuJ6YJSUcEBjpRHJU4fy+PQPB3haF-92cRO87dFQ@mail.gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.1801182254470.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
On Thu, Jan 18, 2018 at 2:00 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 6 quoted lines
>> + 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.
Show 11 quoted lines
>
>> @@ -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