Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Remi Galan Alfonso writes: > At first we wanted a clear indication about removing a commit in > rebase -i. We didn't know about the noop command. > - 'noop' does the same thing as 'drop' but for the user deleting a >commit through 'noop' doesn't seem to be the proper way to use this >command. Moreover 'noop' is not documented (especially it is not >shown in the short help that is display in the editor during the >rebase -i) As Matthieu said elsewhere during the discussion, 'noop' is merely to ignore everything that follows (so 'noop stupid git' does not mean "ignore commit identified by 'stupid git'") and the only reason why it exists is as a hack to have a way to make a non-empty todo list, because an empty todo list means "do not do anything, not even rewinding to the onto commit". As such, I actually think it is a mistake to consider 'noop' as "drop _this_ commit"; the remainder of the line that begins with 'noop' does not name any commit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
At first we wanted a clear indication about removing a commit in rebase -i. We didn't know about the noop command. - 'noop' does the same thing as 'drop' but for the user deleting a commit through 'noop' doesn't seem to be the proper way to use this command. Moreover 'noop' is not documented (especially it is not shown in the short help that is display in the editor during the rebase -i) - Commenting the line does the same but will cause problems with the second part of the patch (warn about removed commits) because the line is then not taken into consideration. We wanted to add the 'drop' command because we wanted a way for clearly deleting a commit (and also for the second part of the patch) and there is no clear way so far. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Johannes Schindelin writes: > Hi Stefan, > > On 2015-05-27 23:47, Stefan Beller wrote: >> On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano wrote: >> Talking about ideas: >> I sometimes have the wrong branch checked out when doing a small >> fixup commit. So I want to drop that patch from the current branch >> and apply it to another branch. Maybe an instruction like >> cherry-pick-to-branch-(and-do-not-apply-here) would help me there. > > Oh, is it wish-anything time? *claps-his-hands* > > I would wish for a graphical tool which visualizes the commit graph in > a visually pleasing manner, where I can select one or more commits and > drop them onto a commit in the graph, and then it goes and magically > cherry-picks-and-drops them. You need to argue a bit more to convince my students to schedule this for the end of their projects ;-). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
On Thu, May 28, 2015 at 10:06 AM, Johannes Schindelin
wrote:
> Hi Stefan,
>
> On 2015-05-27 23:47, Stefan Beller wrote:
>> On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano wrote:
>> Talking about ideas:
>> I sometimes have the wrong branch checked out when doing a small
>> fixup commit. So I want to drop that patch from the current branch
>> and apply it to another branch. Maybe an instruction like
>> cherry-pick-to-branch-(and-do-not-apply-here) would help me there.
>
> Oh, is it wish-anything time? *claps-his-hands*
Maybe my wording was bad, sorry about that.
I think throwing around ideas (which are closely related to what
is trying to be accomplished here IMHO) is not necessarily bad,
but the exchange of ideas helps in understanding the issue better
("I like your idea as I have not thought about it that way.", "What about
use case X", Your idea is nuts because Y")
>
> I would wish for a graphical tool which visualizes the commit graph in a
> visually pleasing manner, where I can select one or more commits and drop
> them onto a commit in the graph, and then it goes and magically
> cherry-picks-and-drops them.
Drag and Drop, I get it. ;)
Additionally, if dropped on an unnamed branch, it should come up with
a reasonable
new branch name.
>
> :-)
>
> Ciao,
> Dscho
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Hi Stefan, On 2015-05-27 23:47, Stefan Beller wrote: > On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano wrote: > Talking about ideas: > I sometimes have the wrong branch checked out when doing a small > fixup commit. So I want to drop that patch from the current branch > and apply it to another branch. Maybe an instruction like > cherry-pick-to-branch-(and-do-not-apply-here) would help me there. Oh, is it wish-anything time? *claps-his-hands* I would wish for a graphical tool which visualizes the commit graph in a visually pleasing manner, where I can select one or more commits and drop them onto a commit in the graph, and then it goes and magically cherry-picks-and-drops them. :-) Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
From: "Junio C Hamano" Matthieu Moy writes: I find it weird to write noop True, but then it can be spelled # too, so do we still want 'drop'? Unless we have a strong reason to believe migrants from Hg cannot be (re)trained, personally, I'd feel that we do not need this 'drop' thing. To me, the addition of "drop" would be a better completion of the list of action verbs for 'normal' users. Training/Retraining users to use atypical techniques is a never ending task, so making drop a synonym for the existing noop appeals to my experience of users (of all sorts of tools, including personal experience ;-). -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano wrote: > Matthieu Moy writes: > >> Junio C Hamano writes: >> >>> Matthieu Moy writes: >>> I find it weird to write noop >>> >>> True, but then it can be spelled >>> >>> # >> >> I do find it weird too. "#" means "comment", which means "do as if it >> was not there" to me. And in this case it does change the semantics once >> you activate the safety feature: error out without the "# >> ", rebase dropping the commit if the comment is present. > > Well, I do not agree with the premise that "A line was removed, the > user may have made a mistake, we need to warn about it" is a good > idea in the first place. Removing an insn that is not wanted has > been the way to skip and not replay a change from the beginning of > the time, and users shouldn't be trained into thinking that somehow > is a bad practice by having such an option that warns. Talking about ideas: I sometimes have the wrong branch checked out when doing a small fixup commit. So I want to drop that patch from the current branch and apply it to another branch. Maybe an instruction like cherry-pick-to-branch-(and-do-not-apply-here) would help me there. On the other hand I do understand the reasoning for having more safety features in rebase as that exposes lots of power and many people find the power a bit daunting. So maybe you don't want to check the rebase instructions, but rather after the fact, when the rebase is done: $ git rebase -i origin/master Successfully rebased and updated refs/heads/mytopic Rebased the following commits: 0e33744 Document protocol version 2 6b6e3a7 t5544: add a test case for the new protocol d6aff73 transport: get_refs_via_connect exchanges capabilities before refs. cbb6089 transport: connect_setup appends protocol version number 0b86fa1 fetch-pack: use the configured transport protocol 23ed0ff remote.h: add get_remote_capabilities, request_capabilities e18b6dc transport: add infrastructure to support a protocol version number fd8d40d upload-pack-2: Implement the version 2 of upload-pack bf781ae upload-pack: move capabilities out of send_ref 4c9cb59 upload-pack: make client capability parsing code a separate function Dropped the following commits: deadbee upload-pack: only accept capabilities on the first "want" line New commits: (due to rewording, double picking, etc) c0ffee1 More Documentation I'd guess you would construct the information from the reflog (The line before "rebase -i (start)" in the reflog) delta'd against HEAD, so it's a crude incantation of git log maybe? Also we need to turn this off for the power users, though I'd welcome if we'd make it default on in git 3. (Being maximally verbose is good for new users I assume, and turning it off is easy for advanced folks, so we can do that for all porcelain commands?) > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Matthieu Moy writes: > Junio C Hamano writes: > >> Matthieu Moy writes: >> >>> I find it weird to write >>> >>> noop >> >> True, but then it can be spelled >> >> # > > I do find it weird too. "#" means "comment", which means "do as if it > was not there" to me. And in this case it does change the semantics once > you activate the safety feature: error out without the "# > ", rebase dropping the commit if the comment is present. Well, I do not agree with the premise that "A line was removed, the user may have made a mistake, we need to warn about it" is a good idea in the first place. Removing an insn that is not wanted has been the way to skip and not replay a change from the beginning of the time, and users shouldn't be trained into thinking that somehow is a bad practice by having such an option that warns. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Junio C Hamano writes: > Matthieu Moy writes: > >> I find it weird to write >> >> noop > > True, but then it can be spelled > > # I do find it weird too. "#" means "comment", which means "do as if it was not there" to me. And in this case it does change the semantics once you activate the safety feature: error out without the "# ", rebase dropping the commit if the comment is present. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Matthieu Moy writes: > I find it weird to write > > noop True, but then it can be spelled # too, so do we still want 'drop'? Unless we have a strong reason to believe migrants from Hg cannot be (re)trained, personally, I'd feel that we do not need this 'drop' thing. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Remi Galan Alfonso writes: > It also has some effects with the second part of this patch (checks > removed and/or duplicated commits): if you comment the line, the > commit will be considered as removed, thus ending in a warning if the > config variable is set to warn/error; however this problem won't > appear with noop. Indeed, that's the whole point of having a "drop" command. As an advice for your next submission: use "git send-email --cover-letter", and explain the overall idea before the patches. I personally prefer "drop" to "noop" as a command name: I understand "noop" as a command without argument (useful to say "this is actually an empty list of commands, not an empty file to ask rebase to abort"), but I find it weird to write noop As Remi wrote, the inspiration comes from Mercurial. Perhaps we should ask on the mercurial ml how happy they are with the name. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Thank you for reviewing the code. Johannes Schindelin writes: > Please note that you can already just comment-out the line if you need to > keep a visual trace. > > Alternatively, you can replace the `pick` command by `noop`. > > If you really need the `drop` command (with which I am not 100% > happy because I already envisage users appending a `drop A` to an > edit script "pick A; pick B; pick C" and expecting A *not to be > picked*) It is true that drop has the same effect as noop or commenting, however we thought that drop is more understandable for average users of git. Moreover when using git rebase -i, the 'help' displayed below the list of commits doesn't mention neither the noop command nor the effect of commenting the line (though considering what removing a line does, it can be easily deduced). The drop command was inspired by the drop command from histedit in mercurial. It also has some effects with the second part of this patch (checks removed and/or duplicated commits): if you comment the line, the commit will be considered as removed, thus ending in a warning if the config variable is set to warn/error; however this problem won't appear with noop. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Hi Rémi,
On 2015-05-26 23:38, Galan Rémi wrote:
> Instead of removing a line to remove the commit, you can use the key
> word "drop" (just like "pick" or "edit"). It has the same effect as
> deleting the line (removing the commit) except that you keep a visual
> trace of your actions, allowing a better control and reducing the
> possibility of removing a commit by mistake.
Please note that you can already just comment-out the line if you need to keep
a visual trace.
Alternatively, you can replace the `pick` command by `noop`.
If you really need the `drop` command (with which I am not 100% happy because I
already envisage users appending a `drop A` to an edit script "pick A; pick B;
pick C" and expecting A *not to be picked*), then it is better to just add the
`drop` part to the already existing `noop` clause:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f7deeb0..8355be8 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -489,7 +489,7 @@ do_next () {
rm -f "$msg" "$author_script" "$amend" || exit
read -r command sha1 rest < "$todo"
case "$command" in
- "$comment_char"*|''|noop)
+ "$comment_char"*|''|noop|drop)
mark_action_done
;;
pick|p)
Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
On Tue, May 26, 2015 at 5:38 PM, Galan Rémi wrote: > git-rebase -i: Add key word "drop" to remove a commit "key word" is unusual. More typical is "keyword". However, perhaps "command" might be even better. Also, custom on this project is not to capitalize, so: git-rebase -i: add command "drop" to remove a commit > Instead of removing a line to remove the commit, you can use the key > word "drop" (just like "pick" or "edit"). It has the same effect as > deleting the line (removing the commit) except that you keep a visual > trace of your actions, allowing a better control and reducing the > possibility of removing a commit by mistake. Nicely explained. Ditto regarding "key word". > Signed-off-by: Galan Rémi > --- > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 1d01baa..3cd2ef2 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -514,6 +514,9 @@ rebasing. > If you just want to edit the commit message for a commit, replace the > command "pick" with the command "reword". > > +If you want to remove a commit, replace the command "pick" by the > +command "drop". I think the existing method of removing a commit merits mention here. Perhaps: To drop a commit, delete its line or replace the command "pick" with "drop". Or, if you want to emphasize "drop": To drop a commit, replace the command "pick" with "drop", or just delete its line. > If you want to fold two or more commits into one, replace the command > "pick" for the second and subsequent commits with "squash" or "fixup". > If the commits had different authors, the folded commit will be > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index dc3133f..cb749e8 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Instead of removing a line to remove the commit, you can use the key
word "drop" (just like "pick" or "edit"). It has the same effect as
deleting the line (removing the commit) except that you keep a visual
trace of your actions, allowing a better control and reducing the
possibility of removing a commit by mistake.
Signed-off-by: Galan Rémi
---
Documentation/git-rebase.txt | 3 +++
git-rebase--interactive.sh| 4
t/lib-rebase.sh | 4 ++--
t/t3404-rebase-interactive.sh | 11 +++
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d01baa..3cd2ef2 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -514,6 +514,9 @@ rebasing.
If you just want to edit the commit message for a commit, replace the
command "pick" with the command "reword".
+If you want to remove a commit, replace the command "pick" by the
+command "drop".
+
If you want to fold two or more commits into one, replace the command
"pick" for the second and subsequent commits with "squash" or "fixup".
If the commits had different authors, the folded commit will be
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..cb749e8 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -152,6 +152,7 @@ Commands:
s, squash = use commit, but meld into previous commit
f, fixup = like "squash", but discard this commit's log message
x, exec = run command (the rest of the line) using shell
+ d, drop = remove commit
These lines can be re-ordered; they are executed from top to bottom.
@@ -515,6 +516,9 @@ do_next () {
do_pick $sha1 "$rest"
record_in_rewritten $sha1
;;
+ drop|d)
+ mark_action_done
+ ;;
reword|r)
comment_for_reflog reword
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6bd2522..fdbc900 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -14,7 +14,7 @@
# specified line.
#
# " " -- add a line with the specified command
-# ("squash", "fixup", "edit", or "reword") and the SHA1 taken
+# ("squash", "fixup", "edit", "reword" or "drop") and the SHA1 taken
# from the specified line.
#
# "exec_cmd_with_args" -- add an "exec cmd with args" line.
@@ -46,7 +46,7 @@ set_fake_editor () {
action=pick
for line in $FAKE_LINES; do
case $line in
- squash|fixup|edit|reword)
+ squash|fixup|edit|reword|drop)
action="$line";;
exec*)
echo "$line" | sed 's/_/ /g' >> "$1";;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ac429a0..1bad068 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,4 +1102,15 @@ test_expect_success 'rebase -i commits that overwrite
untracked files (no ff)' '
test $(git cat-file commit HEAD | sed -ne \$p) = I
'
+test_expect_success 'drop' '
+ git checkout master &&
+ test_when_finished "git checkout master" &&
+ git checkout -b dropBranchTest master &&
+ set_fake_editor &&
+ FAKE_LINES="1 drop 2 3 drop 4 5" git rebase -i --root &&
+ test E = $(git cat-file commit HEAD | sed -ne \$p) &&
+ test C = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+ test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
+'
+
test_done
--
2.4.1.174.g28bfe8e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
