* --no-edit not respected after conflict @ 2021-03-19 14:44 Renato Botelho 2021-03-19 21:30 ` brian m. carlson 0 siblings, 1 reply; 8+ messages in thread From: Renato Botelho @ 2021-03-19 14:44 UTC (permalink / raw) To: git I was reverting multiple commits using --no-edit parameter and after one of those commits conflicted and I resolved using mergetool, no-edit option was not respected anymore and next commits opened editor for me to review commit message. -- Renato Botelho ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-19 14:44 --no-edit not respected after conflict Renato Botelho @ 2021-03-19 21:30 ` brian m. carlson 2021-03-22 13:03 ` Renato Botelho 0 siblings, 1 reply; 8+ messages in thread From: brian m. carlson @ 2021-03-19 21:30 UTC (permalink / raw) To: Renato Botelho; +Cc: git [-- Attachment #1: Type: text/plain, Size: 639 bytes --] On 2021-03-19 at 14:44:30, Renato Botelho wrote: > I was reverting multiple commits using --no-edit parameter and after one of > those commits conflicted and I resolved using mergetool, no-edit option was > not respected anymore and next commits opened editor for me to review commit > message. I'm not sure I understand what you're seeing here, and I think maybe if I knew that I could provide more useful information. Could you maybe provide the set of commands that you're running up to and when you see this problem, or even better, a reproduction testcase? -- brian m. carlson (he/him or they/them) Houston, Texas, US [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-19 21:30 ` brian m. carlson @ 2021-03-22 13:03 ` Renato Botelho 2021-03-22 17:14 ` Elijah Newren 0 siblings, 1 reply; 8+ messages in thread From: Renato Botelho @ 2021-03-22 13:03 UTC (permalink / raw) To: brian m. carlson, git On 19/03/21 18:30, brian m. carlson wrote: > On 2021-03-19 at 14:44:30, Renato Botelho wrote: >> I was reverting multiple commits using --no-edit parameter and after one of >> those commits conflicted and I resolved using mergetool, no-edit option was >> not respected anymore and next commits opened editor for me to review commit >> message. > > I'm not sure I understand what you're seeing here, and I think maybe if > I knew that I could provide more useful information. Could you maybe > provide the set of commands that you're running up to and when you see > this problem, or even better, a reproduction testcase? > I ran `git revert --no-edit commit1 commit2 ... commitN` and one of those reverts had a conflict and the process stopped waiting for a resolution. I ran `git mergetool` and resolved the conflict, then ran `git revert --continue` and then it ignored --no-edit parameter for all other commits and opened $EDITOR for me to edit commit message. I managed to reproduce it on a testing repository doing following steps: % echo a > file % git init % git add file % git commit -m a % echo b > file; git commit -a -m b % echo c > file; git commit -a -m c % echo d > file; git commit -a -m d % echo e > file; git commit -a -m e % git log --oneline d3ec7fc e 23ad2b7 d 2265c82 c 5e0c98a b b34f81a a % git revert --no-edit d3ec7fc 2265c82 5e0c98a It will revert d3ec7fc without any interaction, as expected, then will stop the process on 2265c82 due to conflict and after resolve conflict when I do: % git revert --continue --no-edit parameter will be ignored when reverting 5e0c98a. -- Renato Botelho ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-22 13:03 ` Renato Botelho @ 2021-03-22 17:14 ` Elijah Newren 2021-03-24 0:59 ` Elijah Newren 0 siblings, 1 reply; 8+ messages in thread From: Elijah Newren @ 2021-03-22 17:14 UTC (permalink / raw) To: Renato Botelho; +Cc: brian m. carlson, Git Mailing List On Mon, Mar 22, 2021 at 6:09 AM Renato Botelho <garga@freebsd.org> wrote: > > On 19/03/21 18:30, brian m. carlson wrote: > > On 2021-03-19 at 14:44:30, Renato Botelho wrote: > >> I was reverting multiple commits using --no-edit parameter and after one of > >> those commits conflicted and I resolved using mergetool, no-edit option was > >> not respected anymore and next commits opened editor for me to review commit > >> message. > > > > I'm not sure I understand what you're seeing here, and I think maybe if > > I knew that I could provide more useful information. Could you maybe > > provide the set of commands that you're running up to and when you see > > this problem, or even better, a reproduction testcase? > > > > I ran `git revert --no-edit commit1 commit2 ... commitN` and one of > those reverts had a conflict and the process stopped waiting for a > resolution. > > I ran `git mergetool` and resolved the conflict, then ran `git revert > --continue` and then it ignored --no-edit parameter for all other > commits and opened $EDITOR for me to edit commit message. > > I managed to reproduce it on a testing repository doing following steps: > > % echo a > file > % git init > % git add file > % git commit -m a > % echo b > file; git commit -a -m b > % echo c > file; git commit -a -m c > % echo d > file; git commit -a -m d > % echo e > file; git commit -a -m e > % git log --oneline > > d3ec7fc e > 23ad2b7 d > 2265c82 c > 5e0c98a b > b34f81a a > > % git revert --no-edit d3ec7fc 2265c82 5e0c98a > > It will revert d3ec7fc without any interaction, as expected, then will > stop the process on 2265c82 due to conflict and after resolve conflict > when I do: > > % git revert --continue > > --no-edit parameter will be ignored when reverting 5e0c98a. Thanks for the testcase. I can reproduce. sequencer.c:save_opts() will only save non-zero values (and since options.edit defaults to 1, it'll only save the default value). sequencer.c:continue_single_pick() was written assuming struct replay_opts was not necessary, so even if opts->edit is 0, it just runs a plain "git commit" anyway. It should include --no-edit --cleanup=strip. I've got a patch that fixes both issues, but need to make a proper testcase and whatnot. Maybe I'll have time to do that tonight. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-22 17:14 ` Elijah Newren @ 2021-03-24 0:59 ` Elijah Newren 2021-03-24 1:27 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: Elijah Newren @ 2021-03-24 0:59 UTC (permalink / raw) To: Renato Botelho; +Cc: brian m. carlson, Git Mailing List On Mon, Mar 22, 2021 at 10:14 AM Elijah Newren <newren@gmail.com> wrote: > > On Mon, Mar 22, 2021 at 6:09 AM Renato Botelho <garga@freebsd.org> wrote: > > > > On 19/03/21 18:30, brian m. carlson wrote: > > > On 2021-03-19 at 14:44:30, Renato Botelho wrote: > > >> I was reverting multiple commits using --no-edit parameter and after one of > > >> those commits conflicted and I resolved using mergetool, no-edit option was > > >> not respected anymore and next commits opened editor for me to review commit > > >> message. > > > > > > I'm not sure I understand what you're seeing here, and I think maybe if > > > I knew that I could provide more useful information. Could you maybe > > > provide the set of commands that you're running up to and when you see > > > this problem, or even better, a reproduction testcase? > > > > > > > I ran `git revert --no-edit commit1 commit2 ... commitN` and one of > > those reverts had a conflict and the process stopped waiting for a > > resolution. > > > > I ran `git mergetool` and resolved the conflict, then ran `git revert > > --continue` and then it ignored --no-edit parameter for all other > > commits and opened $EDITOR for me to edit commit message. > > > > I managed to reproduce it on a testing repository doing following steps: > > > > % echo a > file > > % git init > > % git add file > > % git commit -m a > > % echo b > file; git commit -a -m b > > % echo c > file; git commit -a -m c > > % echo d > file; git commit -a -m d > > % echo e > file; git commit -a -m e > > % git log --oneline > > > > d3ec7fc e > > 23ad2b7 d > > 2265c82 c > > 5e0c98a b > > b34f81a a > > > > % git revert --no-edit d3ec7fc 2265c82 5e0c98a > > > > It will revert d3ec7fc without any interaction, as expected, then will > > stop the process on 2265c82 due to conflict and after resolve conflict > > when I do: > > > > % git revert --continue > > > > --no-edit parameter will be ignored when reverting 5e0c98a. > > Thanks for the testcase. I can reproduce. > > sequencer.c:save_opts() will only save non-zero values (and since > options.edit defaults to 1, it'll only save the default value). > > sequencer.c:continue_single_pick() was written assuming struct > replay_opts was not necessary, so even if opts->edit is 0, it just > runs a plain "git commit" anyway. It should include --no-edit > --cleanup=strip. > > I've got a patch that fixes both issues, but need to make a proper > testcase and whatnot. Maybe I'll have time to do that tonight. This turns out to be messier than I expected, and I still don't know what correct behavior is for two related cases. As best I've figured, current behavior and expected behavior is as follows (note the question marks for two entries in the expected behavior table): === Current behavior === Non-conflict commits Right after Conflict revert Edit iff isatty(0) Edit (ignore isatty(0)) cherry-pick No edit See above Specify --edit Edit (ignore isatty(0)) See above Specify --no-edit (*) See above (*) Before stopping for conflicts, No edit is the behavior. After stopping for conflicts, the --no-edit flag is not saved so see the first two rows. === Expected behavior === Non-conflict commits Right after Conflict revert Edit iff isatty(0) Edit (regardless of isatty(0)?) cherry-pick No edit Edit (regardless of isatty(0)?) Specify --edit Edit (ignore isatty(0)) Edit (ignore isatty(0)) Specify --no-edit No edit No edit The thing I'm unsure on is the !isatty(0) handling for revert & cherry-pick right after a conflict when neither --edit nor --no-edit are specified. Should that attempt to launch an editor, which is likely to fail? It does currently, but given that it ignored --no-edit previously it may have just been an oversight rather than intentional behavior. Thoughts? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-24 0:59 ` Elijah Newren @ 2021-03-24 1:27 ` Junio C Hamano 2021-03-26 7:19 ` Elijah Newren 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2021-03-24 1:27 UTC (permalink / raw) To: Elijah Newren; +Cc: Renato Botelho, brian m. carlson, Git Mailing List Elijah Newren <newren@gmail.com> writes: > === Current behavior === > Non-conflict commits Right after Conflict > revert Edit iff isatty(0) Edit (ignore isatty(0)) > cherry-pick No edit See above > Specify --edit Edit (ignore isatty(0)) See above > Specify --no-edit (*) See above > > (*) Before stopping for conflicts, No edit is the behavior. After > stopping for conflicts, the --no-edit flag is not saved so see the > first two rows. > > === Expected behavior === > > Non-conflict commits Right after Conflict > revert Edit iff isatty(0) Edit (regardless of isatty(0)?) > cherry-pick No edit Edit (regardless of isatty(0)?) > Specify --edit Edit (ignore isatty(0)) Edit (ignore isatty(0)) > Specify --no-edit No edit No edit > > The thing I'm unsure on is the !isatty(0) handling for revert & > cherry-pick right after a conflict when neither --edit nor --no-edit > are specified. I read the intention behind existing "edit if isatty" as "this is an operation the human reader deserves a chance to explain what was done and why by default". For example, I read the first entry in your table as: Even if there is no conflict, there should be a convincing explanation when you revert. On the other hand, if you are cherry-picking without any conflict, the intention should be clear enough in the original commit log message, which ought to be written why applying that change is a good idea, so it would make sense not to invoke editor in that case. If an operation deserves a chance to be explained even in a cleanly auto resolved case, it does deserve the chance even more if hand resolution was required---in addition to the original "what and why", the resolution of the conflict is an additional reason why the human should be given a chance to explain. But if it is an automated process, there is no reason to fail the operation merely because the process is run unattended. So my recommendation for "regardless of isatty" part is "do not force editing". The same is true for a human user who declines the chance to explain him/herself with an explicit "--no-edit". ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-24 1:27 ` Junio C Hamano @ 2021-03-26 7:19 ` Elijah Newren 2021-03-26 15:36 ` Renato Botelho 0 siblings, 1 reply; 8+ messages in thread From: Elijah Newren @ 2021-03-26 7:19 UTC (permalink / raw) To: Junio C Hamano; +Cc: Renato Botelho, brian m. carlson, Git Mailing List On Tue, Mar 23, 2021 at 6:27 PM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > > === Current behavior === > > Non-conflict commits Right after Conflict > > revert Edit iff isatty(0) Edit (ignore isatty(0)) > > cherry-pick No edit See above > > Specify --edit Edit (ignore isatty(0)) See above > > Specify --no-edit (*) See above > > > > (*) Before stopping for conflicts, No edit is the behavior. After > > stopping for conflicts, the --no-edit flag is not saved so see the > > first two rows. > > > > === Expected behavior === > > > > Non-conflict commits Right after Conflict > > revert Edit iff isatty(0) Edit (regardless of isatty(0)?) > > cherry-pick No edit Edit (regardless of isatty(0)?) > > Specify --edit Edit (ignore isatty(0)) Edit (ignore isatty(0)) > > Specify --no-edit No edit No edit > > > > The thing I'm unsure on is the !isatty(0) handling for revert & > > cherry-pick right after a conflict when neither --edit nor --no-edit > > are specified. > > I read the intention behind existing "edit if isatty" as "this is an > operation the human reader deserves a chance to explain what was > done and why by default". For example, I read the first entry in > your table as: Even if there is no conflict, there should be a > convincing explanation when you revert. On the other hand, if you > are cherry-picking without any conflict, the intention should be > clear enough in the original commit log message, which ought to be > written why applying that change is a good idea, so it would make > sense not to invoke editor in that case. > > If an operation deserves a chance to be explained even in a cleanly > auto resolved case, it does deserve the chance even more if hand > resolution was required---in addition to the original "what and > why", the resolution of the conflict is an additional reason why the > human should be given a chance to explain. > > But if it is an automated process, there is no reason to fail the > operation merely because the process is run unattended. So my > recommendation for "regardless of isatty" part is "do not force > editing". The same is true for a human user who declines the chance > to explain him/herself with an explicit "--no-edit". Thanks. Renato: potential fix over here: https://lore.kernel.org/git/pull.988.git.git.1616742969145.gitgitgadget@gmail.com/T/#u. Could you give it a try? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: --no-edit not respected after conflict 2021-03-26 7:19 ` Elijah Newren @ 2021-03-26 15:36 ` Renato Botelho 0 siblings, 0 replies; 8+ messages in thread From: Renato Botelho @ 2021-03-26 15:36 UTC (permalink / raw) To: Elijah Newren, Junio C Hamano; +Cc: brian m. carlson, Git Mailing List On 26/03/21 04:19, Elijah Newren wrote: > On Tue, Mar 23, 2021 at 6:27 PM Junio C Hamano <gitster@pobox.com> wrote: >> >> Elijah Newren <newren@gmail.com> writes: >> >>> === Current behavior === >>> Non-conflict commits Right after Conflict >>> revert Edit iff isatty(0) Edit (ignore isatty(0)) >>> cherry-pick No edit See above >>> Specify --edit Edit (ignore isatty(0)) See above >>> Specify --no-edit (*) See above >>> >>> (*) Before stopping for conflicts, No edit is the behavior. After >>> stopping for conflicts, the --no-edit flag is not saved so see the >>> first two rows. >>> >>> === Expected behavior === >>> >>> Non-conflict commits Right after Conflict >>> revert Edit iff isatty(0) Edit (regardless of isatty(0)?) >>> cherry-pick No edit Edit (regardless of isatty(0)?) >>> Specify --edit Edit (ignore isatty(0)) Edit (ignore isatty(0)) >>> Specify --no-edit No edit No edit >>> >>> The thing I'm unsure on is the !isatty(0) handling for revert & >>> cherry-pick right after a conflict when neither --edit nor --no-edit >>> are specified. >> >> I read the intention behind existing "edit if isatty" as "this is an >> operation the human reader deserves a chance to explain what was >> done and why by default". For example, I read the first entry in >> your table as: Even if there is no conflict, there should be a >> convincing explanation when you revert. On the other hand, if you >> are cherry-picking without any conflict, the intention should be >> clear enough in the original commit log message, which ought to be >> written why applying that change is a good idea, so it would make >> sense not to invoke editor in that case. >> >> If an operation deserves a chance to be explained even in a cleanly >> auto resolved case, it does deserve the chance even more if hand >> resolution was required---in addition to the original "what and >> why", the resolution of the conflict is an additional reason why the >> human should be given a chance to explain. >> >> But if it is an automated process, there is no reason to fail the >> operation merely because the process is run unattended. So my >> recommendation for "regardless of isatty" part is "do not force >> editing". The same is true for a human user who declines the chance >> to explain him/herself with an explicit "--no-edit". > > Thanks. > > Renato: potential fix over here: > https://lore.kernel.org/git/pull.988.git.git.1616742969145.gitgitgadget@gmail.com/T/#u. > Could you give it a try? It worked! Thanks! -- Renato Botelho ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-26 15:38 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-19 14:44 --no-edit not respected after conflict Renato Botelho 2021-03-19 21:30 ` brian m. carlson 2021-03-22 13:03 ` Renato Botelho 2021-03-22 17:14 ` Elijah Newren 2021-03-24 0:59 ` Elijah Newren 2021-03-24 1:27 ` Junio C Hamano 2021-03-26 7:19 ` Elijah Newren 2021-03-26 15:36 ` Renato Botelho
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).