* [PATCH] rebase -i: add config to abbreviate command name @ 2017-04-24 3:23 Liam Beguin 2017-04-24 10:26 ` Johannes Schindelin ` (3 more replies) 0 siblings, 4 replies; 28+ messages in thread From: Liam Beguin @ 2017-04-24 3:23 UTC (permalink / raw) To: git; +Cc: liambeguin, martin.von.zweigbergk Add the 'rebase.abbrevCmd' boolean config option to allow the user to abbreviate the default command name while editing the 'git-rebase-todo' file. Signed-off-by: Liam Beguin <liambeguin@gmail.com> --- Notes: * This allows the lines to remain aligned when using single letter commands. Documentation/config.txt | 3 +++ Documentation/git-rebase.txt | 3 +++ git-rebase--interactive.sh | 8 ++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 475e874d5155..59b64832aeb4 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2614,6 +2614,9 @@ rebase.instructionFormat:: the instruction list during an interactive rebase. The format will automatically have the long commit hash prepended to the format. +rebase.abbrevCmd:: + If set to true, abbreviate command name in interactive mode. + receive.advertiseAtomic:: By default, git-receive-pack will advertise the atomic push capability to its clients. If you don't want to advertise this diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 67d48e688315..0c423d903625 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -222,6 +222,9 @@ rebase.missingCommitsCheck:: rebase.instructionFormat:: Custom commit list format to use during an `--interactive` rebase. +rebase.abbrevCmd:: + If set to true, abbreviate command name in interactive mode. + OPTIONS ------- --onto <newbase>:: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 2c9c0165b5ab..9f3e82b79615 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1210,6 +1210,10 @@ else revisions=$onto...$orig_head shortrevisions=$shorthead fi + +rebasecmd=pick +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p + format=$(git config --get rebase.instructionFormat) # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse git rev-list $merges_option --format="%m%H ${format:-%s}" \ @@ -1228,7 +1232,7 @@ do if test t != "$preserve_merges" then - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" else if test -z "$rebase_root" then @@ -1246,7 +1250,7 @@ do if test f = "$preserve" then touch "$rewritten"/$sha1 - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" fi fi done -- 2.9.3 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] rebase -i: add config to abbreviate command name 2017-04-24 3:23 [PATCH] rebase -i: add config to abbreviate command name Liam Beguin @ 2017-04-24 10:26 ` Johannes Schindelin 2017-04-24 11:04 ` liam BEGUIN 2017-04-25 2:57 ` liam BEGUIN 2017-04-24 12:29 ` Jeff King ` (2 subsequent siblings) 3 siblings, 2 replies; 28+ messages in thread From: Johannes Schindelin @ 2017-04-24 10:26 UTC (permalink / raw) To: Liam Beguin; +Cc: git, martin.von.zweigbergk Hi Liam, On Sun, 23 Apr 2017, Liam Beguin wrote: > Add the 'rebase.abbrevCmd' boolean config option to allow > the user to abbreviate the default command name while editing > the 'git-rebase-todo' file. This patch does not handle the `git rebase --edit-todo` subcommand. Intentional? Ciao, Johannes ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] rebase -i: add config to abbreviate command name 2017-04-24 10:26 ` Johannes Schindelin @ 2017-04-24 11:04 ` liam BEGUIN 2017-04-25 2:57 ` liam BEGUIN 1 sibling, 0 replies; 28+ messages in thread From: liam BEGUIN @ 2017-04-24 11:04 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, martin.von.zweigbergk Hi, On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote: > Hi Liam, > > On Sun, 23 Apr 2017, Liam Beguin wrote: > > > Add the 'rebase.abbrevCmd' boolean config option to allow > > the user to abbreviate the default command name while editing > > the 'git-rebase-todo' file. > > This patch does not handle the `git rebase --edit-todo` subcommand. > Intentional? no, this is not intentional, I'll make the changes. > > Ciao, > Johannes Thanks, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] rebase -i: add config to abbreviate command name 2017-04-24 10:26 ` Johannes Schindelin 2017-04-24 11:04 ` liam BEGUIN @ 2017-04-25 2:57 ` liam BEGUIN 2017-04-25 19:45 ` Johannes Schindelin 1 sibling, 1 reply; 28+ messages in thread From: liam BEGUIN @ 2017-04-25 2:57 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git Hi Johannes, On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote: > Hi Liam, > > On Sun, 23 Apr 2017, Liam Beguin wrote: > > > Add the 'rebase.abbrevCmd' boolean config option to allow > > the user to abbreviate the default command name while editing > > the 'git-rebase-todo' file. > > This patch does not handle the `git rebase --edit-todo` subcommand. > Intentional? After a little more investigation, I'm not sure what should be added for the `git rebase --edit-todo` subcommand. It seems like it uses the same text that was added the first time (with `git rebase -i`). Do you have a bit more information about what you meant? I don't use this subcommand very often, I'm most likely missing something. > > Ciao, > Johannes Thanks, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] rebase -i: add config to abbreviate command name 2017-04-25 2:57 ` liam BEGUIN @ 2017-04-25 19:45 ` Johannes Schindelin 2017-04-25 22:58 ` liam BEGUIN 0 siblings, 1 reply; 28+ messages in thread From: Johannes Schindelin @ 2017-04-25 19:45 UTC (permalink / raw) To: liam BEGUIN; +Cc: git Hi Liam, On Mon, 24 Apr 2017, liam BEGUIN wrote: > On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote: > > > On Sun, 23 Apr 2017, Liam Beguin wrote: > > > > > Add the 'rebase.abbrevCmd' boolean config option to allow the user > > > to abbreviate the default command name while editing the > > > 'git-rebase-todo' file. > > > > This patch does not handle the `git rebase --edit-todo` subcommand. > > Intentional? > > After a little more investigation, I'm not sure what should be added for > the `git rebase --edit-todo` subcommand. It seems like it uses the same > text that was added the first time (with `git rebase -i`). Well, it uses whatever the user may have edited. It may surprise users that their `pick` does not get converted to `p` like all the original commands. Ciao, Johannes ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] rebase -i: add config to abbreviate command name 2017-04-25 19:45 ` Johannes Schindelin @ 2017-04-25 22:58 ` liam BEGUIN 0 siblings, 0 replies; 28+ messages in thread From: liam BEGUIN @ 2017-04-25 22:58 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git Hi Johannes, On Tue, 2017-04-25 at 21:45 +0200, Johannes Schindelin wrote: > Hi Liam, > > On Mon, 24 Apr 2017, liam BEGUIN wrote: > > > On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote: > > > > > On Sun, 23 Apr 2017, Liam Beguin wrote: > > > > > > > Add the 'rebase.abbrevCmd' boolean config option to allow the user > > > > to abbreviate the default command name while editing the > > > > 'git-rebase-todo' file. > > > > > > This patch does not handle the `git rebase --edit-todo` subcommand. > > > Intentional? > > > > After a little more investigation, I'm not sure what should be added for > > the `git rebase --edit-todo` subcommand. It seems like it uses the same > > text that was added the first time (with `git rebase -i`). > > Well, it uses whatever the user may have edited. It may surprise users > that their `pick` does not get converted to `p` like all the original > commands. > It makes more sens to me now, I'll add it in next patch > Ciao, > Johannes Thanks, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] rebase -i: add config to abbreviate command name 2017-04-24 3:23 [PATCH] rebase -i: add config to abbreviate command name Liam Beguin 2017-04-24 10:26 ` Johannes Schindelin @ 2017-04-24 12:29 ` Jeff King 2017-04-25 4:37 ` [PATCH v2] rebase -i: add config to abbreviate command-names Liam Beguin 2017-04-25 4:43 ` Liam Beguin 3 siblings, 0 replies; 28+ messages in thread From: Jeff King @ 2017-04-24 12:29 UTC (permalink / raw) To: Liam Beguin; +Cc: git, martin.von.zweigbergk On Sun, Apr 23, 2017 at 11:23:47PM -0400, Liam Beguin wrote: > Add the 'rebase.abbrevCmd' boolean config option to allow > the user to abbreviate the default command name while editing > the 'git-rebase-todo' file. Just reading this, I was confused about what the patch actually did. Reading the code, I figured it out, but perhaps an example would make sense. Like: This means that we will print: p 1234abcd subject line in the todo file rather than: pick 1234abcd subject line And then of course that left me wondering why somebody would want to do that. I understand wanting to _type_ the abbreviated version, but surely it's not too much work to read the full word? Then I saw: > --- > Notes: > > * This allows the lines to remain aligned when using single > letter commands. That makes some sense. it should probably be part of the commit message, so that future readers of "git log" understand why the change was made. > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 475e874d5155..59b64832aeb4 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2614,6 +2614,9 @@ rebase.instructionFormat:: > the instruction list during an interactive rebase. The format will automatically > have the long commit hash prepended to the format. > > +rebase.abbrevCmd:: > + If set to true, abbreviate command name in interactive mode. Similar to the commit message, this might need to go into more detail. It was not immediately obvious to me that "command name" means the command-names in the instruction list. -Peff ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-24 3:23 [PATCH] rebase -i: add config to abbreviate command name Liam Beguin 2017-04-24 10:26 ` Johannes Schindelin 2017-04-24 12:29 ` Jeff King @ 2017-04-25 4:37 ` Liam Beguin 2017-04-25 6:29 ` Junio C Hamano ` (2 more replies) 2017-04-25 4:43 ` Liam Beguin 3 siblings, 3 replies; 28+ messages in thread From: Liam Beguin @ 2017-04-25 4:37 UTC (permalink / raw) To: git; +Cc: Jhannes.Schindelin, peff, Liam Beguin Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` to abbreviate the command-names in the instruction list. This means that `git rebase -i` would print: p deadbee The oneline of this commit ... instead of: pick deadbee The oneline of this commit ... Using a single character command-name allows the lines to remain aligned, making the whole set more readable. Signed-off-by: Liam Beguin <liambeguin@gmail.com> --- Changes since v1: - Improve Documentation and commit message Documentation/config.txt | 19 +++++++++++++++++++ Documentation/git-rebase.txt | 19 +++++++++++++++++++ git-rebase--interactive.sh | 8 ++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 475e874d5155..8b1877f2df91 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: the instruction list during an interactive rebase. The format will automatically have the long commit hash prepended to the format. +rebase.abbrevCmd:: + If set to true, `git rebase -i` will abbreviate the command-names in the + instruction list. This means that instead of looking like this, + +------------------------------------------- + pick deadbee The oneline of this commit + pick fa1afe1 The oneline of the next commit + ... +------------------------------------------- + + the list would use the short version of the command resulting in + something like this. + +------------------------------------------- + p deadbee The oneline of this commit + p fa1afe1 The oneline of the next commit + ... +------------------------------------------- + receive.advertiseAtomic:: By default, git-receive-pack will advertise the atomic push capability to its clients. If you don't want to advertise this diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 67d48e688315..7d97c0483241 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -222,6 +222,25 @@ rebase.missingCommitsCheck:: rebase.instructionFormat:: Custom commit list format to use during an `--interactive` rebase. +rebase.abbrevCmd:: + If set to true, `git rebase -i` will abbreviate the command-names in the + instruction list. This means that instead of looking like this, + +------------------------------------------- + pick deadbee The oneline of this commit + pick fa1afe1 The oneline of the next commit + ... +------------------------------------------- + + the list would use the short version of the command resulting in + something like this. + +------------------------------------------- + p deadbee The oneline of this commit + p fa1afe1 The oneline of the next commit + ... +------------------------------------------- + OPTIONS ------- --onto <newbase>:: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 2c9c0165b5ab..9f3e82b79615 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1210,6 +1210,10 @@ else revisions=$onto...$orig_head shortrevisions=$shorthead fi + +rebasecmd=pick +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p + format=$(git config --get rebase.instructionFormat) # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse git rev-list $merges_option --format="%m%H ${format:-%s}" \ @@ -1228,7 +1232,7 @@ do if test t != "$preserve_merges" then - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" else if test -z "$rebase_root" then @@ -1246,7 +1250,7 @@ do if test f = "$preserve" then touch "$rewritten"/$sha1 - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" fi fi done -- 2.9.3 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 4:37 ` [PATCH v2] rebase -i: add config to abbreviate command-names Liam Beguin @ 2017-04-25 6:29 ` Junio C Hamano 2017-04-25 8:29 ` Jacob Keller 2017-04-25 9:57 ` Andreas Schwab 2017-04-25 10:34 ` Philip Oakley 2 siblings, 1 reply; 28+ messages in thread From: Junio C Hamano @ 2017-04-25 6:29 UTC (permalink / raw) To: Liam Beguin; +Cc: git, Jhannes.Schindelin, peff Liam Beguin <liambeguin@gmail.com> writes: > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > to abbreviate the command-names in the instruction list. > > This means that `git rebase -i` would print: > p deadbee The oneline of this commit > ... > > instead of: > pick deadbee The oneline of this commit > ... Whenever I see "This means that...", my automatic reaction is "The author expects what s/he wrote previously is not understandable, and is making another try to give something more readable. As this is not a real time communication, why not rewrite the incomprehensible part before wasting the time of the readers by throwing at them what is known to the author to be unreadble, only to clarify with 'This means that...' later?" But I think in this case, you do not even have to say "This means that". What you wrote, without "This means that", i.e. Add the 'rebase.abbrevCommand' configuration variable to tell `git rebase -i` to show commands abbreviated in the instruction list, i.e. p deadbee The oneline of this commit ... instead of: pick deadbee The oneline of this commit ... is quite readable. > Using a single character command-name allows the lines to remain > aligned, making the whole set more readable. Hmph. I have trouble with "lines remain aligned". Depending on the object names of commits, don't you end up getting something like this that is not aligned? p deadbee The oneline p e2cb6ab8 Another commit Or are you happy with only the beginning of object names aligned, without the actual titles aligned? Personally I am happy with the beginning of each instruction line aligned, so from that point of view, this patch is a mild Meh to me, even though I do a fair amount of "rebase -i" myself. But obviously I am not the only user of Git you need to please, so... ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 6:29 ` Junio C Hamano @ 2017-04-25 8:29 ` Jacob Keller 2017-04-25 23:34 ` liam Beguin 2017-04-26 2:09 ` Junio C Hamano 0 siblings, 2 replies; 28+ messages in thread From: Jacob Keller @ 2017-04-25 8:29 UTC (permalink / raw) To: Junio C Hamano Cc: Liam Beguin, Git mailing list, Jhannes.Schindelin, Jeff King On Mon, Apr 24, 2017 at 11:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > Personally I am happy with the beginning of each instruction line > aligned, so from that point of view, this patch is a mild Meh to me, > even though I do a fair amount of "rebase -i" myself. But obviously > I am not the only user of Git you need to please, so... I would instead justify this as making it easier to change the action, since you only need to rewrite a single letter, which at least in vim takes "r<letter>" to change the action, vs slightly more keystrokes such as "ct <letter" or otherwise. Also, if you change the default commit hash length, it becomes long enough to cover most commits and you see all commits at say 12 digits commit hash and everything is nicely aligned. Thanks, Jake ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 8:29 ` Jacob Keller @ 2017-04-25 23:34 ` liam Beguin 2017-04-26 2:09 ` Junio C Hamano 1 sibling, 0 replies; 28+ messages in thread From: liam Beguin @ 2017-04-25 23:34 UTC (permalink / raw) To: Jacob Keller, Junio C Hamano Cc: Git mailing list, Jhannes.Schindelin, Jeff King Hi Jake, On Tue, 2017-04-25 at 01:29 -0700, Jacob Keller wrote: > On Mon, Apr 24, 2017 at 11:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > > Personally I am happy with the beginning of each instruction line > > aligned, so from that point of view, this patch is a mild Meh to me, > > even though I do a fair amount of "rebase -i" myself. But obviously > > I am not the only user of Git you need to please, so... > > I would instead justify this as making it easier to change the action, > since you only need to rewrite a single letter, which at least in vim > takes "r<letter>" to change the action, vs slightly more keystrokes > such as "ct <letter" or otherwise. It's another reason that motivated the change but I didn't think the vim shortcuts would justify the patch. Since you pointed it out, I'll probably add it. > > Also, if you change the default commit hash length, it becomes long > enough to cover most commits and you see all commits at say 12 digits > commit hash and everything is nicely aligned. > > Thanks, > Jake Thanks, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 8:29 ` Jacob Keller 2017-04-25 23:34 ` liam Beguin @ 2017-04-26 2:09 ` Junio C Hamano 1 sibling, 0 replies; 28+ messages in thread From: Junio C Hamano @ 2017-04-26 2:09 UTC (permalink / raw) To: Jacob Keller; +Cc: Liam Beguin, Git mailing list, Jhannes.Schindelin, Jeff King Jacob Keller <jacob.keller@gmail.com> writes: > On Mon, Apr 24, 2017 at 11:29 PM, Junio C Hamano <gitster@pobox.com> wrote: >> Personally I am happy with the beginning of each instruction line >> aligned, so from that point of view, this patch is a mild Meh to me, >> even though I do a fair amount of "rebase -i" myself. But obviously >> I am not the only user of Git you need to please, so... > > I would instead justify this as making it easier to change the action, > since you only need to rewrite a single letter, which at least in vim > takes "r<letter>" to change the action, vs slightly more keystrokes > such as "ct <letter" or otherwise. That makes sense to me too. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 4:37 ` [PATCH v2] rebase -i: add config to abbreviate command-names Liam Beguin 2017-04-25 6:29 ` Junio C Hamano @ 2017-04-25 9:57 ` Andreas Schwab 2017-04-25 13:59 ` Mike Rappazzo 2017-04-25 10:34 ` Philip Oakley 2 siblings, 1 reply; 28+ messages in thread From: Andreas Schwab @ 2017-04-25 9:57 UTC (permalink / raw) To: Liam Beguin; +Cc: git, Jhannes.Schindelin, peff On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote: > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > to abbreviate the command-names in the instruction list. > > This means that `git rebase -i` would print: > p deadbee The oneline of this commit > ... > > instead of: > pick deadbee The oneline of this commit > ... > > Using a single character command-name allows the lines to remain > aligned, making the whole set more readable. Perhaps there should rather be an option to tell rebase to align the columns? Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 9:57 ` Andreas Schwab @ 2017-04-25 13:59 ` Mike Rappazzo 0 siblings, 0 replies; 28+ messages in thread From: Mike Rappazzo @ 2017-04-25 13:59 UTC (permalink / raw) To: Andreas Schwab; +Cc: Liam Beguin, Git List, Jhannes.Schindelin, Jeff King On Tue, Apr 25, 2017 at 5:57 AM, Andreas Schwab <schwab@linux-m68k.org> wrote: > On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote: > >> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` >> to abbreviate the command-names in the instruction list. >> >> This means that `git rebase -i` would print: >> p deadbee The oneline of this commit >> ... >> >> instead of: >> pick deadbee The oneline of this commit >> ... >> >> Using a single character command-name allows the lines to remain >> aligned, making the whole set more readable. > > Perhaps there should rather be an option to tell rebase to align the > columns? > You _can_ set a custom instruction format using the config variable: `rebase.instructionFormat`. With this, you can align columns using the normal git log format. For example, I personally use this as my instruction format: [%an%<|(64)%x5d %s While, this won't always align perfectly, it may help scratch your itch. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 4:37 ` [PATCH v2] rebase -i: add config to abbreviate command-names Liam Beguin 2017-04-25 6:29 ` Junio C Hamano 2017-04-25 9:57 ` Andreas Schwab @ 2017-04-25 10:34 ` Philip Oakley 2 siblings, 0 replies; 28+ messages in thread From: Philip Oakley @ 2017-04-25 10:34 UTC (permalink / raw) To: Liam Beguin, git; +Cc: Jhannes.Schindelin, peff, Liam Beguin From: "Liam Beguin" <liambeguin@gmail.com> > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > to abbreviate the command-names in the instruction list. > > This means that `git rebase -i` would print: > p deadbee The oneline of this commit > ... > > instead of: > pick deadbee The oneline of this commit > ... > > Using a single character command-name allows the lines to remain > aligned, making the whole set more readable. > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> > --- > Changes since v1: > - Improve Documentation and commit message > > Documentation/config.txt | 19 +++++++++++++++++++ > Documentation/git-rebase.txt | 19 +++++++++++++++++++ > git-rebase--interactive.sh | 8 ++++++-- > 3 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 475e874d5155..8b1877f2df91 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: > the instruction list during an interactive rebase. The format will > automatically > have the long commit hash prepended to the format. > > +rebase.abbrevCmd:: > + If set to true, `git rebase -i` will abbreviate the command-names in the > + instruction list. This means that instead of looking like this, > + > +------------------------------------------- > + pick deadbee The oneline of this commit > + pick fa1afe1 The oneline of the next commit > + ... > +------------------------------------------- > + > + the list would use the short version of the command resulting in > + something like this. Perhaps use an example which does have rebase commands of different lengths, such as 'pick', 'squash', 'reword' to more clearly show the intent of alignment and subsequent ease of editing? -- Philip > + > +------------------------------------------- > + p deadbee The oneline of this commit > + p fa1afe1 The oneline of the next commit > + ... > +------------------------------------------- > + > receive.advertiseAtomic:: > By default, git-receive-pack will advertise the atomic push > capability to its clients. If you don't want to advertise this > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 67d48e688315..7d97c0483241 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -222,6 +222,25 @@ rebase.missingCommitsCheck:: > rebase.instructionFormat:: > Custom commit list format to use during an `--interactive` rebase. > > +rebase.abbrevCmd:: > + If set to true, `git rebase -i` will abbreviate the command-names in the > + instruction list. This means that instead of looking like this, > + > +------------------------------------------- > + pick deadbee The oneline of this commit > + pick fa1afe1 The oneline of the next commit > + ... > +------------------------------------------- > + > + the list would use the short version of the command resulting in > + something like this. > + > +------------------------------------------- > + p deadbee The oneline of this commit > + p fa1afe1 The oneline of the next commit > + ... > +------------------------------------------- > + > OPTIONS > ------- > --onto <newbase>:: > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 2c9c0165b5ab..9f3e82b79615 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -1210,6 +1210,10 @@ else > revisions=$onto...$orig_head > shortrevisions=$shorthead > fi > + > +rebasecmd=pick > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p > + > format=$(git config --get rebase.instructionFormat) > # the 'rev-list .. | sed' requires %m to parse; the instruction requires > %H to parse > git rev-list $merges_option --format="%m%H ${format:-%s}" \ > @@ -1228,7 +1232,7 @@ do > > if test t != "$preserve_merges" > then > - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" > + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" > else > if test -z "$rebase_root" > then > @@ -1246,7 +1250,7 @@ do > if test f = "$preserve" > then > touch "$rewritten"/$sha1 > - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" > + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" > fi > fi > done > -- > 2.9.3 > > ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-24 3:23 [PATCH] rebase -i: add config to abbreviate command name Liam Beguin ` (2 preceding siblings ...) 2017-04-25 4:37 ` [PATCH v2] rebase -i: add config to abbreviate command-names Liam Beguin @ 2017-04-25 4:43 ` Liam Beguin 2017-04-25 9:53 ` Andreas Schwab ` (2 more replies) 3 siblings, 3 replies; 28+ messages in thread From: Liam Beguin @ 2017-04-25 4:43 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, peff, Liam Beguin Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` to abbreviate the command-names in the instruction list. This means that `git rebase -i` would print: p deadbee The oneline of this commit ... instead of: pick deadbee The oneline of this commit ... Using a single character command-name allows the lines to remain aligned, making the whole set more readable. Signed-off-by: Liam Beguin <liambeguin@gmail.com> --- Changes since v1: - Improve Documentation and commit message Documentation/config.txt | 19 +++++++++++++++++++ Documentation/git-rebase.txt | 19 +++++++++++++++++++ git-rebase--interactive.sh | 8 ++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 475e874d5155..8b1877f2df91 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: the instruction list during an interactive rebase. The format will automatically have the long commit hash prepended to the format. +rebase.abbrevCmd:: + If set to true, `git rebase -i` will abbreviate the command-names in the + instruction list. This means that instead of looking like this, + +------------------------------------------- + pick deadbee The oneline of this commit + pick fa1afe1 The oneline of the next commit + ... +------------------------------------------- + + the list would use the short version of the command resulting in + something like this. + +------------------------------------------- + p deadbee The oneline of this commit + p fa1afe1 The oneline of the next commit + ... +------------------------------------------- + receive.advertiseAtomic:: By default, git-receive-pack will advertise the atomic push capability to its clients. If you don't want to advertise this diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 67d48e688315..7d97c0483241 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -222,6 +222,25 @@ rebase.missingCommitsCheck:: rebase.instructionFormat:: Custom commit list format to use during an `--interactive` rebase. +rebase.abbrevCmd:: + If set to true, `git rebase -i` will abbreviate the command-names in the + instruction list. This means that instead of looking like this, + +------------------------------------------- + pick deadbee The oneline of this commit + pick fa1afe1 The oneline of the next commit + ... +------------------------------------------- + + the list would use the short version of the command resulting in + something like this. + +------------------------------------------- + p deadbee The oneline of this commit + p fa1afe1 The oneline of the next commit + ... +------------------------------------------- + OPTIONS ------- --onto <newbase>:: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 2c9c0165b5ab..9f3e82b79615 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1210,6 +1210,10 @@ else revisions=$onto...$orig_head shortrevisions=$shorthead fi + +rebasecmd=pick +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p + format=$(git config --get rebase.instructionFormat) # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse git rev-list $merges_option --format="%m%H ${format:-%s}" \ @@ -1228,7 +1232,7 @@ do if test t != "$preserve_merges" then - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" else if test -z "$rebase_root" then @@ -1246,7 +1250,7 @@ do if test f = "$preserve" then touch "$rewritten"/$sha1 - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" fi fi done -- 2.9.3 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 4:43 ` Liam Beguin @ 2017-04-25 9:53 ` Andreas Schwab 2017-04-25 21:23 ` Johannes Schindelin 2017-04-25 20:08 ` Johannes Schindelin 2017-04-26 15:24 ` Ævar Arnfjörð Bjarmason 2 siblings, 1 reply; 28+ messages in thread From: Andreas Schwab @ 2017-04-25 9:53 UTC (permalink / raw) To: Liam Beguin; +Cc: git, Johannes.Schindelin, peff On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote: > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 475e874d5155..8b1877f2df91 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: > the instruction list during an interactive rebase. The format will automatically > have the long commit hash prepended to the format. > > +rebase.abbrevCmd:: > + If set to true, `git rebase -i` will abbreviate the command-names in the > + instruction list. This means that instead of looking like this, > + > +------------------------------------------- > + pick deadbee The oneline of this commit > + pick fa1afe1 The oneline of the next commit > + ... > +------------------------------------------- > + > + the list would use the short version of the command resulting in > + something like this. > + > +------------------------------------------- > + p deadbee The oneline of this commit > + p fa1afe1 The oneline of the next commit > + ... > +------------------------------------------- That doesn't explain the point of the option. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 9:53 ` Andreas Schwab @ 2017-04-25 21:23 ` Johannes Schindelin 2017-04-25 22:56 ` liam BEGUIN 0 siblings, 1 reply; 28+ messages in thread From: Johannes Schindelin @ 2017-04-25 21:23 UTC (permalink / raw) To: Andreas Schwab; +Cc: Liam Beguin, git, peff Hi Andreas, On Tue, 25 Apr 2017, Andreas Schwab wrote: > On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote: > > > diff --git a/Documentation/config.txt b/Documentation/config.txt > > index 475e874d5155..8b1877f2df91 100644 > > --- a/Documentation/config.txt > > +++ b/Documentation/config.txt > > @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: > > the instruction list during an interactive rebase. The format will automatically > > have the long commit hash prepended to the format. > > > > +rebase.abbrevCmd:: > > + If set to true, `git rebase -i` will abbreviate the command-names in the > > + instruction list. This means that instead of looking like this, > > + > > +------------------------------------------- > > + pick deadbee The oneline of this commit > > + pick fa1afe1 The oneline of the next commit > > + ... > > +------------------------------------------- > > + > > + the list would use the short version of the command resulting in > > + something like this. > > + > > +------------------------------------------- > > + p deadbee The oneline of this commit > > + p fa1afe1 The oneline of the next commit > > + ... > > +------------------------------------------- > > That doesn't explain the point of the option. And what you forgot to say in order to make this a constructive criticism is: we probably want to add a sentence like this: Using the one-letter abbreviations will align the lines better in case that the non-abbreviated commands have different lengths. Speaking of commands with different lengths, I just thought of fixup and squash. I do not think those are handled by the patch, but they should be (the `action` in the first loop of `rearrange_squash` should abbreviate via `test p != "$pickcmd" || action=${action%${action#?}}`). Ciao, Johannes ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 21:23 ` Johannes Schindelin @ 2017-04-25 22:56 ` liam BEGUIN 0 siblings, 0 replies; 28+ messages in thread From: liam BEGUIN @ 2017-04-25 22:56 UTC (permalink / raw) To: Johannes Schindelin, Andreas Schwab; +Cc: git, peff Hi Johannes, On Tue, 2017-04-25 at 23:23 +0200, Johannes Schindelin wrote: > Hi Andreas, > > On Tue, 25 Apr 2017, Andreas Schwab wrote: > > > On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote: > > > > > diff --git a/Documentation/config.txt b/Documentation/config.txt > > > index 475e874d5155..8b1877f2df91 100644 > > > --- a/Documentation/config.txt > > > +++ b/Documentation/config.txt > > > @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: > > > the instruction list during an interactive rebase. The format will automatically > > > have the long commit hash prepended to the format. > > > > > > +rebase.abbrevCmd:: > > > + If set to true, `git rebase -i` will abbreviate the command-names in the > > > + instruction list. This means that instead of looking like this, > > > + > > > +------------------------------------------- > > > + pick deadbee The oneline of this commit > > > + pick fa1afe1 The oneline of the next commit > > > + ... > > > +------------------------------------------- > > > + > > > + the list would use the short version of the command resulting in > > > + something like this. > > > + > > > +------------------------------------------- > > > + p deadbee The oneline of this commit > > > + p fa1afe1 The oneline of the next commit > > > + ... > > > +------------------------------------------- > > > > That doesn't explain the point of the option. > > And what you forgot to say in order to make this a constructive criticism > is: we probably want to add a sentence like this: > > > Using the one-letter abbreviations will align the lines better > in case that the non-abbreviated commands have different lengths. > > Speaking of commands with different lengths, I just thought of fixup and > squash. I do not think those are handled by the patch, but they should be > (the `action` in the first loop of `rearrange_squash` should abbreviate > via `test p != "$pickcmd" || action=${action%${action#?}}`). > I just noticed this today, I'll make changes to handle this case. > Ciao, > Johannes Thanks, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 4:43 ` Liam Beguin 2017-04-25 9:53 ` Andreas Schwab @ 2017-04-25 20:08 ` Johannes Schindelin 2017-04-26 0:13 ` liam Beguin 2017-04-26 15:24 ` Ævar Arnfjörð Bjarmason 2 siblings, 1 reply; 28+ messages in thread From: Johannes Schindelin @ 2017-04-25 20:08 UTC (permalink / raw) To: Liam Beguin; +Cc: git, peff Hi Liam, On Tue, 25 Apr 2017, Liam Beguin wrote: > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > to abbreviate the command-names in the instruction list. > > This means that `git rebase -i` would print: > p deadbee The oneline of this commit > ... > > instead of: > pick deadbee The oneline of this commit > ... > > Using a single character command-name allows the lines to remain > aligned, making the whole set more readable. > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> Apart from either abbreviating commands after --edit-todo, or documenting explicitly that the new config option only concerns the initial todo list, there is another problem that just occurred to me: --exec. When you call `git rebase -x "make DEVELOPER=1 -j15"`, the idea is to append an "exec make DEVELOPER=1 -j15" line after every pick line. The code in question looks like this: add_exec_commands () { { first=t while read -r insn rest do case $insn in pick) test -n "$first" || printf "%s" "$cmd" ;; esac printf "%s %s\n" "$insn" "$rest" first= done printf "%s" "$cmd" } <"$1" >"$1.new" && mv "$1.new" "$1" } Obviously, the git-rebase--interactive script expects at this point that the command is spelled out, so your patch needs to change the `pick)` case to `p|pick)`, I think. In addition, since the rationale for the new option is to align the lines better, the `exec` would need to be replaced by `x`, and as multiple `-x` options are allowed, you would need something like this at the beginning of `add_exec_commands`, too: # abbreviate `exec` if rebase.abbrevCmd is true test p != "$rebasecmd" || cmd="$(echo "$cmd" | sed 's/^exec/x/')" Also: > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 475e874d5155..8b1877f2df91 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: > the instruction list during an interactive rebase. The format will automatically > have the long commit hash prepended to the format. > > +rebase.abbrevCmd:: It does not fail to amuse that the term "abbrevCmd" is abbreviated heavily itself. However, I would strongly suggest to avoid that. It would be much more pleasant to call the config option rebase.abbreviateCommands > +rebase.abbrevCmd:: > + If set to true, `git rebase -i` will abbreviate the command-names in the > + instruction list. This means that instead of looking like this, This is by no means your fault, but it is really horrible by how many different names Git's documentation refers to the todo script, nothing short of confusing. It is the todo script (which I called it initially, maybe not a good name, but it has the merit of the longest tradition at least), the todo list, the instruction sheet, the rebase script, the instruction list... etc However, the thing is called "todo list" elsewhere in the same file, therefore lets try to avoid even more confusion and use that term instead of "instruction list" here. > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 2c9c0165b5ab..9f3e82b79615 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -1210,6 +1210,10 @@ else > revisions=$onto...$orig_head > shortrevisions=$shorthead > fi > + > +rebasecmd=pick > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p A better name would be "pickcmd", as there are more rebase commands than just `pick` and what we want here is really only associated with one of those commands. Ciao, Johannes ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 20:08 ` Johannes Schindelin @ 2017-04-26 0:13 ` liam Beguin 2017-04-26 1:47 ` Jeff King 2017-04-26 9:28 ` Johannes Schindelin 0 siblings, 2 replies; 28+ messages in thread From: liam Beguin @ 2017-04-26 0:13 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, peff Hi Johannes, On Tue, 2017-04-25 at 22:08 +0200, Johannes Schindelin wrote: > Hi Liam, > > On Tue, 25 Apr 2017, Liam Beguin wrote: > > > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > > to abbreviate the command-names in the instruction list. > > > > This means that `git rebase -i` would print: > > p deadbee The oneline of this commit > > ... > > > > instead of: > > pick deadbee The oneline of this commit > > ... > > > > Using a single character command-name allows the lines to remain > > aligned, making the whole set more readable. > > > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> > > Apart from either abbreviating commands after --edit-todo, or documenting > explicitly that the new config option only concerns the initial todo list, > there is another problem that just occurred to me: --exec. > > When you call `git rebase -x "make DEVELOPER=1 -j15"`, the idea is to > append an "exec make DEVELOPER=1 -j15" line after every pick line. The > code in question looks like this: > > add_exec_commands () { > { > first=t > while read -r insn rest > do > case $insn in > pick) > test -n "$first" || > printf "%s" "$cmd" > ;; > esac > printf "%s %s\n" "$insn" "$rest" > first= > done > printf "%s" "$cmd" > } <"$1" >"$1.new" && > mv "$1.new" "$1" > } > > Obviously, the git-rebase--interactive script expects at this point that > the command is spelled out, so your patch needs to change the `pick)` case > to `p|pick)`, I think. > > In addition, since the rationale for the new option is to align the lines > better, the `exec` would need to be replaced by `x`, and as multiple `-x` > options are allowed, you would need something like this at the beginning > of `add_exec_commands`, too: > > # abbreviate `exec` if rebase.abbrevCmd is true > test p != "$rebasecmd" || > cmd="$(echo "$cmd" | sed 's/^exec/x/')" > > Also: > > > diff --git a/Documentation/config.txt b/Documentation/config.txt > > index 475e874d5155..8b1877f2df91 100644 > > --- a/Documentation/config.txt > > +++ b/Documentation/config.txt > > @@ -2614,6 +2614,25 @@ rebase.instructionFormat:: > > the instruction list during an interactive rebase. The format will automatically > > have the long commit hash prepended to the format. > > > > +rebase.abbrevCmd:: > > It does not fail to amuse that the term "abbrevCmd" is abbreviated > heavily itself. However, I would strongly suggest to avoid that. It would > be much more pleasant to call the config option rebase.abbreviateCommands I tried to use something similar to the rest of the options but I guess that would be best. > > > +rebase.abbrevCmd:: > > + If set to true, `git rebase -i` will abbreviate the command-names in the > > + instruction list. This means that instead of looking like this, > > This is by no means your fault, but it is really horrible by how many > different names Git's documentation refers to the todo script, nothing > short of confusing. It is the todo script (which I called it initially, > maybe not a good name, but it has the merit of the longest tradition at > least), the todo list, the instruction sheet, the rebase script, the > instruction list... etc > > However, the thing is called "todo list" elsewhere in the same file, > therefore lets try to avoid even more confusion and use that term instead > of "instruction list" here. thanks for pointing this out, I was not quite sure what to call this list. > > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > > index 2c9c0165b5ab..9f3e82b79615 100644 > > --- a/git-rebase--interactive.sh > > +++ b/git-rebase--interactive.sh > > @@ -1210,6 +1210,10 @@ else > > revisions=$onto...$orig_head > > shortrevisions=$shorthead > > fi > > + > > +rebasecmd=pick > > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p > > A better name would be "pickcmd", as there are more rebase commands than > just `pick` and what we want here is really only associated with one of > those commands. Wouldn't that make it confusing when the patch starts to handle other commands? A common name across the script would limit further confusion. I noticed that it is already called `action` in `rearrange_squash`. would that do? (even though it has no reference to 'command') > > Ciao, > Johannes Thanks for the detailed answer, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-26 0:13 ` liam Beguin @ 2017-04-26 1:47 ` Jeff King 2017-04-26 3:59 ` Junio C Hamano 2017-04-26 9:28 ` Johannes Schindelin 1 sibling, 1 reply; 28+ messages in thread From: Jeff King @ 2017-04-26 1:47 UTC (permalink / raw) To: liam Beguin; +Cc: Johannes Schindelin, git On Tue, Apr 25, 2017 at 08:13:27PM -0400, liam Beguin wrote: > > > +rebase.abbrevCmd:: > > > + If set to true, `git rebase -i` will abbreviate the command-names in the > > > + instruction list. This means that instead of looking like this, > > > > This is by no means your fault, but it is really horrible by how many > > different names Git's documentation refers to the todo script, nothing > > short of confusing. It is the todo script (which I called it initially, > > maybe not a good name, but it has the merit of the longest tradition at > > least), the todo list, the instruction sheet, the rebase script, the > > instruction list... etc > > > > However, the thing is called "todo list" elsewhere in the same file, > > therefore lets try to avoid even more confusion and use that term instead > > of "instruction list" here. > > thanks for pointing this out, I was not quite sure what to call this list. I think the words "instruction list" may have come from my suggestion. I used them because that is the term used in the rebase.instructionFormat documentation directly above the option you are adding. It may be worth a follow-on patch to convert that one to "todo list" if that's the preferred name. -Peff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-26 1:47 ` Jeff King @ 2017-04-26 3:59 ` Junio C Hamano 2017-04-26 9:25 ` Johannes Schindelin 0 siblings, 1 reply; 28+ messages in thread From: Junio C Hamano @ 2017-04-26 3:59 UTC (permalink / raw) To: Jeff King; +Cc: liam Beguin, Johannes Schindelin, git Jeff King <peff@peff.net> writes: > I think the words "instruction list" may have come from my suggestion. I > used them because that is the term used in the rebase.instructionFormat > documentation directly above the option you are adding. > > It may be worth a follow-on patch to convert that one to "todo list" if > that's the preferred name. Running $ git grep -i -e 'instruction [ls]' -e 'todo l' lets us count how we call them, and we can see there is only one instance of 'instruction list'. Running the above in v1.7.3 tree shows that it was originally called 'todo list', and we can see that an enhancement of cherry-pick in cd4093b6 ("Merge branch 'rr/revert-cherry-pick-continue'", 2011-10-05)) started calling this instruction sheet around v1.7.8. A follow-on patch to unify all three would be nice, indeed. Thanks. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-26 3:59 ` Junio C Hamano @ 2017-04-26 9:25 ` Johannes Schindelin 2017-04-27 0:37 ` Junio C Hamano 0 siblings, 1 reply; 28+ messages in thread From: Johannes Schindelin @ 2017-04-26 9:25 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, liam Beguin, git Hi Junio, On Tue, 25 Apr 2017, Junio C Hamano wrote: > Running > > $ git grep -i -e 'instruction [ls]' -e 'todo l' > > lets us count how we call them, and we can see there is only one > instance of 'instruction list'. > > Running the above in v1.7.3 tree shows that it was originally called > 'todo list', and we can see that an enhancement of cherry-pick in > cd4093b6 ("Merge branch 'rr/revert-cherry-pick-continue'", > 2011-10-05)) started calling this instruction sheet around v1.7.8. > > A follow-on patch to unify all three would be nice, indeed. But we cannot unify them, as the config option's name uses "instruction" and to keep backwards-compatibility, we are simply unable to resolve the confusion. Ciao, Dscho ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-26 9:25 ` Johannes Schindelin @ 2017-04-27 0:37 ` Junio C Hamano 0 siblings, 0 replies; 28+ messages in thread From: Junio C Hamano @ 2017-04-27 0:37 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Jeff King, liam Beguin, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Hi Junio, > > On Tue, 25 Apr 2017, Junio C Hamano wrote: > >> Running >> >> $ git grep -i -e 'instruction [ls]' -e 'todo l' >> >> lets us count how we call them, and we can see there is only one >> instance of 'instruction list'. >> >> Running the above in v1.7.3 tree shows that it was originally called >> 'todo list', and we can see that an enhancement of cherry-pick in >> cd4093b6 ("Merge branch 'rr/revert-cherry-pick-continue'", >> 2011-10-05)) started calling this instruction sheet around v1.7.8. >> >> A follow-on patch to unify all three would be nice, indeed. > > But we cannot unify them, as the config option's name uses "instruction" > and to keep backwards-compatibility, we are simply unable to resolve the > confusion. We can correct historical mistakes by introducing preferred synonym to misnamed configuration variables, clearly document why we prefer it over the misnamed one that is now deprecated, and then eventually dropping it at a major version boundary. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-26 0:13 ` liam Beguin 2017-04-26 1:47 ` Jeff King @ 2017-04-26 9:28 ` Johannes Schindelin 1 sibling, 0 replies; 28+ messages in thread From: Johannes Schindelin @ 2017-04-26 9:28 UTC (permalink / raw) To: liam Beguin; +Cc: git, peff [-- Attachment #1: Type: text/plain, Size: 1335 bytes --] Hi Liam, On Tue, 25 Apr 2017, liam Beguin wrote: > On Tue, 2017-04-25 at 22:08 +0200, Johannes Schindelin wrote: > > > > On Tue, 25 Apr 2017, Liam Beguin wrote: > > > > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > > > index 2c9c0165b5ab..9f3e82b79615 100644 > > > --- a/git-rebase--interactive.sh > > > +++ b/git-rebase--interactive.sh > > > @@ -1210,6 +1210,10 @@ else > > > revisions=$onto...$orig_head > > > shortrevisions=$shorthead > > > fi > > > + > > > +rebasecmd=pick > > > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p > > > > A better name would be "pickcmd", as there are more rebase commands than > > just `pick` and what we want here is really only associated with one of > > those commands. > > Wouldn't that make it confusing when the patch starts to handle other > commands? Only if you use that variable to hold other values than `pick` or `p`. But you do not plan on that, right? You plan to use this variable only to hold the value `pick` by default and `p` in case the user asked for abbreviated commands. Therefore, I think it makes sense to reflect in the variable name that the purpose is really only to reflect the string used for the `pick` command (as opposed to any other todo command). Ciao, Johannes ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-25 4:43 ` Liam Beguin 2017-04-25 9:53 ` Andreas Schwab 2017-04-25 20:08 ` Johannes Schindelin @ 2017-04-26 15:24 ` Ævar Arnfjörð Bjarmason 2017-04-27 1:20 ` liam Beguin 2 siblings, 1 reply; 28+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-04-26 15:24 UTC (permalink / raw) To: Liam Beguin; +Cc: Git Mailing List, Johannes Schindelin, Jeff King On Tue, Apr 25, 2017 at 6:43 AM, Liam Beguin <liambeguin@gmail.com> wrote: > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > to abbreviate the command-names in the instruction list. > > This means that `git rebase -i` would print: > p deadbee The oneline of this commit > ... > > instead of: > pick deadbee The oneline of this commit > ... > > Using a single character command-name allows the lines to remain > aligned, making the whole set more readable. Aside from the existing comments about the commit message from others, you should be noting that we *already* have these abbreviations for all the todo list options, and we note this in append_todo_help. > +rebase.abbrevCmd:: > + If set to true, `git rebase -i` will abbreviate the command-names in the > + instruction list. This means that instead of looking like this, > + > [...] > +rebase.abbrevCmd:: > + If set to true, `git rebase -i` will abbreviate the command-names in the > + instruction list. This means that instead of looking like this, > [...] Better to split this out into a new *.txt file and use the include::* facility (grep for it) rather than copy/pasting this entirely across two files. > OPTIONS > ------- > --onto <newbase>:: > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 2c9c0165b5ab..9f3e82b79615 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -1210,6 +1210,10 @@ else > revisions=$onto...$orig_head > shortrevisions=$shorthead > fi > + > +rebasecmd=pick > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p Rather than hardcoding "p" here maybe it would be worthhwile to make that into a variable used both here and in append_todo_help, maybe not... > format=$(git config --get rebase.instructionFormat) > # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse > git rev-list $merges_option --format="%m%H ${format:-%s}" \ > @@ -1228,7 +1232,7 @@ do > > if test t != "$preserve_merges" > then > - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" > + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" > else > if test -z "$rebase_root" > then > @@ -1246,7 +1250,7 @@ do > if test f = "$preserve" > then > touch "$rewritten"/$sha1 > - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" > + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" > fi > fi > done I haven't tried applying & running this patch, but it seems you definitely missed the case where --autosquash will add fixup or squash, that should be f or s with your patch, but you didn't change that code. See the rearrange_squash function. Ditto for turning "exec" into "e" with --exec. But if the motivation for this entire thing is to make sure the commands are aligned this doesn't fix that, because the sha1s can be of different lengths. So as others have pointed out maybe this entire thing should be dropped & replaced with some bool command to align the todo list, maybe turning that on by default. Unless the real unstated reason is to make this easier to edit in vim or something, in which case this approach seems reasonable. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rebase -i: add config to abbreviate command-names 2017-04-26 15:24 ` Ævar Arnfjörð Bjarmason @ 2017-04-27 1:20 ` liam Beguin 0 siblings, 0 replies; 28+ messages in thread From: liam Beguin @ 2017-04-27 1:20 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Git Mailing List, Johannes Schindelin, Jeff King Hi Ævar, On Wed, 2017-04-26 at 17:24 +0200, Ævar Arnfjörð Bjarmason wrote: > On Tue, Apr 25, 2017 at 6:43 AM, Liam Beguin <liambeguin@gmail.com> wrote: > > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > > to abbreviate the command-names in the instruction list. > > > > This means that `git rebase -i` would print: > > p deadbee The oneline of this commit > > ... > > > > instead of: > > pick deadbee The oneline of this commit > > ... > > > > Using a single character command-name allows the lines to remain > > aligned, making the whole set more readable. > > Aside from the existing comments about the commit message from others, > you should be noting that we *already* have these abbreviations for > all the todo list options, and we note this in append_todo_help. > > > > +rebase.abbrevCmd:: > > + If set to true, `git rebase -i` will abbreviate the command-names in the > > + instruction list. This means that instead of looking like this, > > + > > [...] > > +rebase.abbrevCmd:: > > + If set to true, `git rebase -i` will abbreviate the command-names in the > > + instruction list. This means that instead of looking like this, > > [...] > > Better to split this out into a new *.txt file and use the include::* > facility (grep for it) rather than copy/pasting this entirely across > two files. > Thanks for pointing this out, I'll update the documentation > > OPTIONS > > ------- > > --onto <newbase>:: > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > > index 2c9c0165b5ab..9f3e82b79615 100644 > > --- a/git-rebase--interactive.sh > > +++ b/git-rebase--interactive.sh > > @@ -1210,6 +1210,10 @@ else > > revisions=$onto...$orig_head > > shortrevisions=$shorthead > > fi > > + > > +rebasecmd=pick > > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p > > Rather than hardcoding "p" here maybe it would be worthhwile to make > that into a variable used both here and in append_todo_help, maybe > not... > I'm not sure I understand, do you mean that the option should also affect the message added by append_todo_help ? > > format=$(git config --get rebase.instructionFormat) > > # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse > > git rev-list $merges_option --format="%m%H ${format:-%s}" \ > > @@ -1228,7 +1232,7 @@ do > > > > if test t != "$preserve_merges" > > then > > - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" > > + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" > > else > > if test -z "$rebase_root" > > then > > @@ -1246,7 +1250,7 @@ do > > if test f = "$preserve" > > then > > touch "$rewritten"/$sha1 > > - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo" > > + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo" > > fi > > fi > > done > > I haven't tried applying & running this patch, but it seems you > definitely missed the case where --autosquash will add fixup or > squash, that should be f or s with your patch, but you didn't change > that code. See the rearrange_squash function. > > Ditto for turning "exec" into "e" with --exec. > I noticed this yesterday, I'll add both cases the next iteration. > But if the motivation for this entire thing is to make sure the > commands are aligned this doesn't fix that, because the sha1s can be > of different lengths. So as others have pointed out maybe this entire > thing should be dropped & replaced with some bool command to align the > todo list, maybe turning that on by default. > > Unless the real unstated reason is to make this easier to edit in vim > or something, in which case this approach seems reasonable. Keeping things aligned was the first motivation but the fact that it also makes changing the action faster is also nice to have. I didn't think it would help justify the patch. The SHA1s not having the same length can easily be 'fixed' by setting a higher value for 'core.abbrev'. Thanks, Liam ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2017-04-27 1:21 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-24 3:23 [PATCH] rebase -i: add config to abbreviate command name Liam Beguin 2017-04-24 10:26 ` Johannes Schindelin 2017-04-24 11:04 ` liam BEGUIN 2017-04-25 2:57 ` liam BEGUIN 2017-04-25 19:45 ` Johannes Schindelin 2017-04-25 22:58 ` liam BEGUIN 2017-04-24 12:29 ` Jeff King 2017-04-25 4:37 ` [PATCH v2] rebase -i: add config to abbreviate command-names Liam Beguin 2017-04-25 6:29 ` Junio C Hamano 2017-04-25 8:29 ` Jacob Keller 2017-04-25 23:34 ` liam Beguin 2017-04-26 2:09 ` Junio C Hamano 2017-04-25 9:57 ` Andreas Schwab 2017-04-25 13:59 ` Mike Rappazzo 2017-04-25 10:34 ` Philip Oakley 2017-04-25 4:43 ` Liam Beguin 2017-04-25 9:53 ` Andreas Schwab 2017-04-25 21:23 ` Johannes Schindelin 2017-04-25 22:56 ` liam BEGUIN 2017-04-25 20:08 ` Johannes Schindelin 2017-04-26 0:13 ` liam Beguin 2017-04-26 1:47 ` Jeff King 2017-04-26 3:59 ` Junio C Hamano 2017-04-26 9:25 ` Johannes Schindelin 2017-04-27 0:37 ` Junio C Hamano 2017-04-26 9:28 ` Johannes Schindelin 2017-04-26 15:24 ` Ævar Arnfjörð Bjarmason 2017-04-27 1:20 ` liam Beguin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.