Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3 15/15] rebase: change the default backend from "am" to "merge"
From: Johannes Schindelin @ 2020-01-12 17:59 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Elijah Newren, Jonathan Nieder, Elijah Newren via GitGitGadget,
	Git Mailing List, Denton Liu, Junio C Hamano, Pavel Roskin,
	Alban Gruin, SZEDER Gábor
In-Reply-To: <052fdedc-2beb-91ab-c5c3-53fb99e64810@gmail.com>

Hi,

On Sat, 11 Jan 2020, Phillip Wood wrote:

> On 11/01/2020 01:16, Elijah Newren wrote:
> >
> > On Fri, Jan 10, 2020 at 3:14 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
> > >
> > > Elijah Newren via GitGitGadget wrote:
> > >
> > >   1. "git rebase --am" does not invoke the post-commit hook, but "git
> > >      rebase --merge" does.  Is this behavior change intended?
> > >
> > >      Noticed because jiri[1] installs a post-commit hook that warns
> > >      about commits on detached HEAD, so this change makes rebases more
> > >      noisy in repositories that were set up using jiri.
>
> Perhaps that hook could learn not to warn if a branch is being rebased?
> git could be more helpful there by having a porcelain option to status
> that prints the branch name if we're rebasing (`git worktree --list`
> shows the branch correctly when it's being rebased but does not (yet - I
> have a patch to do it) mark the current worktree so isn't very helpful.)
>
> > I've never used a post-commit hook or seen one in the wild.  Certainly
> > wasn't intentional, but it's not clear to me if it's wrong or right
> > either.  I don't see why it would make sense to distinguish between
> > any of git rebase --am/--merge/--interactive, but it isn't too
> > surprising that by historical accident the two rebase backends which
> > happened to call git-commit behind the scenes would call a post-commit
> > hook and the other rebase backend that didn't call git-commit
> > wouldn't.
>
> Looking through the history the am based rebase has never run the post-commit
> hook as am has its own set of hooks and the scripted version used commit-tree.
> The merge based rebase ran `git commit` which ran the post commit hook. The
> interactive rebase ran the hook until and I broke it in a356ee4659b
> ("sequencer: try to commit without forking 'git commit'", 2017-11-24) and
> after I fixed it in 4627bc777e ("sequencer: run post-commit hook",
> 2019-10-15). As it was broken for two years with no one noticing it can't be
> that popular.

Maybe a crazy idea, but maybe not: how about running the `post-commit`
hook _only_ if `--merge` was specified explicitly, and in that case (and
guarded behind a check verifying that the `post-commit` hook _actually_
exists _and_ is executable) warn the user that this hook won't be run in
future versions?

To make things better for users who actually want to run that hook during
rebases, we could introduce a config option, say,
`rebase.runPostCommitHook` that is a tri-state (`true`, `false`,
`onlyForDashDashMerge`, at first defaulting to the last, eventually to
`false`).

Crazy? Or helpful?

> > But the big question here, is what is correct behavior?  Should rebase
> > call the post-commit hook, or should it skip it?  I haven't any clue
> > what the answer to that is.
>
> It's creating a new commit so I lean towards thinking it should run the
> post-commit hook. As an example I have a post-commit hook that prints a
> warning if a commit is created on a branch that is being rewritten by
> one of my scripts in another worktree. There are pre-commit and
> pre-rebase hooks to try and prevent that, but the warning is there as a
> last resort if those hooks are by-passed.

I guess you're right, it is quite surprising that the `post-commit` hook
is _not_ run for `--am` rebases even though commits are created.

> > >   2. GIT_REFLOG_ACTION contains "rebase -i" even though the rebase is
> > >      not interactive.
>
> If this is important to people I think it should be easy enough to set
> GIT_REFLOG_ACTION to the appropriate string in builtin/rebase.c (so long
> as it hasn't already been set by the user) rather than relying on
> sequencer.c to do it.

I agree (but won't have time to implement it, so maybe I should shut up
already...)

> > Yep, as does --keep, --exec, --rebase-merges, etc.  There are lots of
> > rebases which use the interactive machinery even if they aren't
> > explicitly interactive.  I've never seen the "-i" in the reflog
> > message defined, but clearly it has always been used whenever the
> > interactive machinery was in play regardless of whether the rebase was
> > interactive.  In that regard, I figured that --merge fit in rather
> > nicely.  (And I noted the fact that reflog messages were different
> > between the backends among the "BEHAVIORAL DIFFERENCES" section of
> > git-rebase.txt).  But if others think we should just drop the -i (much
> > as we did for the bash prompt), I'd be happy with that too.  If we go
> > that route, I think I'd rather drop the -i in the reflog for all
> > rebases, not just the
> > using-the-interactive-machinery-but-not-explicitly-interactive ones.
> >
> > >   3. In circumstances I haven't pinned down yet, we get the error
> > >      message "invalid date format: @@2592000 +0000":
> > >
> > >          $ git rebase --committer-date-is-author-date --onto branch_K
> > >          branch_L~1 branch_L
> > >          $ git checkout --theirs file
> > >          $ git add file
> > >          $ git rebase --continue
> > >          fatal: invalid date format: @@2592000 +0000
> > >          error: could not commit staged changes.
> > >
> > >      This isn't reproducible without --committer-date-is-author-date.
> > >      More context (the test where it happens) is in [2].
> >
> > Interesting.  Do you happen to know if this started happening with
> > ra/rebase-i-more-options, or did it just become an issue with
> > en/rebase-backend?  I looked around at the link you provided and feel
> > a bit confused; I'm not sure which test does this or how I'd
> > reproduce.
>
> I'm confused by the test as well. As ra/rebase-i-more-options only touched the
> sequencer then any bugs would only show up in this test (which runs a
> non-interactive rebase) once en/rbease-backend switched to that backend. It
> seems likely that ra/rebase-i-more-options is to blame.
>
> Jonathan - do you happen to know if your users create empty commits at all?
> and if so what do they expect rebase to do with them (and any that become
> empty when they are rebased) - cf
> https://lore.kernel.org/git/<CABPp-BEH=9qejeqysHYE+AJ+JPaBympZizq-bx_OjArYFa4xUQ@mail.gmail.com>

The double `@` looks very funny. I would be interested in seeing an MCVE.

> > >   4. I suspect the exit status in the "you need to resolve conflicts"
> > >      case has changed.  With rebase --am, [3] would automatically
> > >      invoke rebase --abort when conflicts are present, but with rebase
> > >      --merge it does not.
> > >
> > > Known?
> >
> > Nope, but I would certainly hope that "you need to resolve conflicts"
> > would result in a non-zero exit status.  If it doesn't, that sounds
> > like a bug in the interactive backend that we need to fix.  I'll dig
> > in.

Yes, exiting with status 0 would be a major bug, and I think it might even
be a bug that was introduced by me when I re-implemented the core loop of
the interactive rebase in C.

But to me it sounds as if 4. is not so about the exit code but about
aborting immediately. I do not recall seeing --am rebases to abort,
though, but to exit with error (and I saw the same behavior in interactive
rebases).

We will need to see a reduced concrete example (preferably as a new test
case) of the described behavior.

Ciao,
Dscho

^ permalink raw reply

* Re: Problems with ra/rebase-i-more-options - should we revert it?
From: Johannes Schindelin @ 2020-01-12 18:41 UTC (permalink / raw)
  To: phillip.wood
  Cc: Junio C Hamano, Git Mailing List, Rohit Ashiwal, Jonathan Nieder
In-Reply-To: <089637d7-b4b6-f6ba-cce1-29e22ce47521@gmail.com>

Hi Phillip,

On Sun, 12 Jan 2020, Phillip Wood wrote:

> On 12/01/2020 16:12, Phillip Wood wrote:
> > I'm concerned that there are some bugs in this series and think
> > it may be best to revert it before releasing 2.25.0. Jonathan
> > Nieder posted a bug report on Friday [1] which I think is caused
> > by this series. While trying to reproduce Jonathan's bug I came
> > up with the test below which fails, but not in the same way.

Thank you so much for your thoughts and your work on this. For what it's
worth, I totally agree with your assessment and your suggestion to revert
those patches _before_ releasing v2.25.0. (I seem to remember vaguely that
there were repeated requests for better test coverage and that those
requests went unaddressed, so I would not be surprised if there were more
unfortunate surprises waiting for us.)

> Doh I forgot to add --committer-date-is-author-date to the rebase
> command line in that test. It passes with that added - how
> embarrassing. However it does appear that it prefixes the date in
> GIT_COMMITTER_DATE with @@ rather than @. I think (though am not
> completely certain yet) the reason the test still passes is that
> the date has more than 8 digits so although
> match_object_header_date() fails because of the '@@'
> match_digit() succeeds once the loop in parse_date_basic() strips
> that prefix. Jonathan's test date only has 7 digits so
> match_digit() does not treat it as a number of seconds since the
> start of the epoch and fails to parse it. The fix for the @@ is
> quite simple, the date we read from the author script already has
> an @ so we don't need to add another. The diff below shows a
> basic fix but we should get rid of datebuf altogether as we don't
> need it. I need a break now I'll try and put a patch together
> later in the week if no one else has by then.

Thank you so much!

>
> Best Wishes
>
> Phillip
>
> --- >8 ---
> diff --git a/sequencer.c b/sequencer.c
> index 763ccbbc45..22a38de47b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -988,7 +988,7 @@ static int run_git_commit(struct repository *r,
>                 if (!date)
>                         return -1;
>
> -               strbuf_addf(&datebuf, "@%s", date);
> +               strbuf_addf(&datebuf, "%s", date);

I have to admit that I have not analyzed the code before this hunk (it
would be much easier to increase the context in a non-static reviewing
environment, e.g. on GitHub, but the mailing list does not allow for
that), so I do not know just _how_ likely our `date` here is going to
change or remain prefixed by a `@`. Therefore, this suggestion might be
totally stupid: `"@%s", date + (*date == '@')`

Thanks again,
Dscho

>                 res = setenv("GIT_COMMITTER_DATE",
>                              opts->ignore_date ? "" : datebuf.buf, 1);
>
> > The
> > test coverage of this series has always been pretty poor and I
> > think it needs improving for us to have confidence in it. I'm
> > also concerned that at least one of the
> > tests ('--committer-date-is-author-date works with rebase -r')
> > does not detect failures properly in the code below
> >
> > 	while read HASH
> > 	do
> > 		git show $HASH --pretty="format:%ai" >authortime
> > 		git show $HASH --pretty="format:%ci" >committertime
> > 		test_cmp authortime committertime
> > 	done <rev_list
> >
> >
> > Best Wishes
> >
> > Phillip
> >
> > [1] https://lore.kernel.org/git/20200110231436.GA24315@google.com/
> >
> > --- >8 ---
> > diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh
> > index 5166f158dd..c81e1d7167 100755
> > --- a/t/t3433-rebase-options-compatibility.sh
> > +++ b/t/t3433-rebase-options-compatibility.sh
> > @@ -6,6 +6,7 @@
> >   test_description='tests to ensure compatibility between am and interactive backends'
> >
> >   . ./test-lib.sh
> > +. "$TEST_DIRECTORY"/lib-rebase.sh
> >
> >   GIT_AUTHOR_DATE="1999-04-02T08:03:20+05:30"
> >   export GIT_AUTHOR_DATE
> > @@ -99,6 +100,22 @@ test_expect_success '--committer-date-is-author-date works with rebase -r' '
> >          done <rev_list
> >   '
> >
> > +test_expect_success '--committer-date-is-author-date works when committing conflict resolution' '
> > +       git checkout commit2 &&
> > +       (
> > +               set_fake_editor &&
> > +               FAKE_LINES=2 &&
> > +               export FAKE_LINES &&
> > +               test_must_fail git rebase -i HEAD^^
> > +       ) &&
> > +       echo resolved > foo &&
> > +       git add foo &&
> > +       git rebase --continue &&
> > +       git log -1 --format=%at commit2 >expect &&
> > +       git log -1 --format=%ct HEAD >actual &&
> > +       test_cmp expect actual
> > +'
> > +
> >   # Checking for +0000 in author time is enough since default
> >   # timezone is UTC, but the timezone used while committing
> >   # sets to +0530.
> >
>

^ permalink raw reply

* [Feature request] An easier way of rebasing if you just want to "force send" a file back to a previous commit
From: 1234dev @ 2020-01-12 19:42 UTC (permalink / raw)
  To: git@vger.kernel.org

Hello!

I'm pretty new to all of this, but I was wondering. Is there an easier way of rebasing if you just want to force send a file back to a previous commit? Rebasing can get quite tiresome in the long run. It's like 7 steps, and that's without the merge conflicts someone with my luck is guaranteed to run into.

For instance, say I've made changes to a file. Those changes are too tiny and insignificant to make a new commit out of - they actually ought to be part of a commit I made last night.

If there just was a way to cheat :) I'm aware it would rewrite my entire history but that's okay, I haven't shared my repo with anybody yet. Maybe something along the lines of "git rebase --off-she-goes <file> <hash>"?

As opposed to "git stash && git rebase --interactive '<hash>^' && <change pick => edit> && <apply changes manually> && git add <file> && git commit --amend && git rebase --continue && git stash pop && <merge conflict that requires manual intervention> && git rebase --continue && git stash pop && <still a conflict> && rm <file> && git checkout <file> && <repeat the whole process> && <still a conflict> && <go to IRC and ask for help>

Might have gotten the above sequence of commands a bit wrong, I just learned how to rebase a few days ago. But hopefully y'all get my point.

Thanks!


Sent with ProtonMail Secure Email.



^ permalink raw reply

* Porting git version 2.25.0.rc2 to hppa2.0w-hp-hpux11.11 using gcc-8.3.1
From: John David Anglin @ 2020-01-12 19:28 UTC (permalink / raw)
  To: git

Since the GCC project is switching to a git archive, there was a need to port git to hpux11.11.
In particular, we need git to continue support for the hppa64 target as linux doesn't yet have a 64-bit
runtime.  This mail documents the changes that I needed to build git on hppa2.0w-hp-hpux11.11.

1) SCNuMAX is missing from inttypes.h

I needed to add back a define in git-compat-util.h.  I will probably fix this in gcc in the near future.

2) strtoll() and strtoull() are not supported

This causes a problem in t/helper/test-progress.c.  Regardless of what configure thinks, the target
supports strtoimax() and strtoumax().  So, I changed t/helper/test-progress.c to use uintmax_t and
strtoumax().  strtoimax() and strtoumax() are used in other places, so this makes test-progress.c
consistent with the other usage.

Internally, strtoumax() is implemented is __strtoull().  So, the strtoumax() version has the same range
as the strtoull() version. It would be possible to implement strtoull() and strtoll() with an include hack
in gcc but most packages use the versions in libiberty and gnulib.

3) Bus error in recv_sideband

See:
http://git.661346.n2.nabble.com/git-failure-on-HP-UX-td6335104.html

This error occurs when one tries to clone an archive:

Dump of assembler code for function recv_sideband:
   0x0017fe30 <+0>:     stw rp,-14(sp)
   0x0017fe34 <+4>:     addil L%10000,sp,r1
   0x0017fe38 <+8>:     ldo 80(r1),sp
=> 0x0017fe3c <+12>:    stw r14,-70(sp)

(gdb) bt
#0  0x0017fe3c in recv_sideband () from /opt/gnu/libexec/git-core/git
#1  0x0012e874 in sideband_demux () from /opt/gnu/libexec/git-core/git
#2  0x001b5e80 in run_thread () from /opt/gnu/libexec/git-core/git
#3  0xc005b290 in __pthread_body () from /opt/langtools/lib/libpthread.1

int recv_sideband(const char *me, int in_stream, int out)
{
        char buf[LARGE_PACKET_MAX + 1];
        ...

The bus error occurs because the frame size needed for buf and the other locals exceeds the
default thread stack size.  This can be changed using the PTHREAD_DEFAULT_STACK_SIZE environment
variable.  For example,

export PTHREAD_DEFAULT_STACK_SIZE=131072

It also could be adjusted using pthread_default_stacksize_np().  However, it seemed better to me to
allocate buf using malloc and avoid the issue entirely.

4) NO_PREAD is required

Without NO_PREAD, we get the following error:

fatal: premature end of pack file, 106 bytes missing
fatal: index-pack failed

This occurs on the first pread() call.  There is some kind of sequencing issue as doing a fprintf
to stderr changes the behavior.  However, it doesn't fix the error.

5) -pthread is required

The libc library contains pthread stub routines.  -pthread is needed to cause gcc to correctly link
with thread support.

The following summarizes the code changes:

diff --git a/git-compat-util.h b/git-compat-util.h
index aed0b5d4f9..bcc0d925bf 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1335,4 +1335,8 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset)
 	((uintptr_t)&(ptr)->member - (uintptr_t)(ptr))
 #endif /* !__GNUC__ */

+#ifndef SCNuMAX
+#define SCNuMAX PRIuMAX
+#endif
+
 #endif
diff --git a/pkt-line.c b/pkt-line.c
index a0e87b1e81..5024325c81 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -444,7 +444,7 @@ ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out)

 int recv_sideband(const char *me, int in_stream, int out)
 {
-	char buf[LARGE_PACKET_MAX + 1];
+	char *buf = xmalloc(LARGE_PACKET_MAX + 1);
 	int len;
 	struct strbuf scratch = STRBUF_INIT;
 	enum sideband_type sideband_type;
@@ -460,6 +460,7 @@ int recv_sideband(const char *me, int in_stream, int out)
 			write_or_die(out, buf + 1, len - 1);
 			break;
 		default: /* errors: message already written */
+			free(buf);
 			return sideband_type;
 		}
 	}
diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c
index 42b96cb103..b96a20237a 100644
--- a/t/helper/test-progress.c
+++ b/t/helper/test-progress.c
@@ -54,18 +54,18 @@ int cmd__progress(int argc, const char **argv)
 		char *end;

 		if (skip_prefix(line.buf, "progress ", (const char **) &end)) {
-			uint64_t item_count = strtoull(end, &end, 10);
+			uintmax_t item_count = strtoumax(end, &end, 10);
 			if (*end != '\0')
 				die("invalid input: '%s'\n", line.buf);
 			display_progress(progress, item_count);
 		} else if (skip_prefix(line.buf, "throughput ",
 				       (const char **) &end)) {
-			uint64_t byte_count, test_ms;
+			uintmax_t byte_count, test_ms;

-			byte_count = strtoull(end, &end, 10);
+			byte_count = strtoumax(end, &end, 10);
 			if (*end != ' ')
 				die("invalid input: '%s'\n", line.buf);
-			test_ms = strtoull(end + 1, &end, 10);
+			test_ms = strtoumax(end + 1, &end, 10);
 			if (*end != '\0')
 				die("invalid input: '%s'\n", line.buf);
 			progress_test_ns = test_ms * 1000 * 1000;

The following summarizes the changes to config.mak.autogen after running configure:

--- config.mak.autogen.save	2020-01-12 13:17:09 +0000
+++ config.mak.autogen	2020-01-12 13:14:54 +0000
@@ -75,13 +75,15 @@
 NO_MEMMEM=YesPlease
 NO_STRLCPY=YesPlease
 NO_UINTMAX_T=
-NO_STRTOUMAX=YesPlease
+NO_STRTOULL=YesPlease
+NO_STRTOUMAX=
 NO_SETENV=YesPlease
 NO_UNSETENV=YesPlease
 NO_MKDTEMP=YesPlease
 NO_INITGROUPS=
 HAVE_GETDELIM=
 HAVE_BSD_SYSCTL=
-PTHREAD_CFLAGS=
+PTHREAD_CFLAGS=-pthread
 PTHREAD_LIBS=
 NO_PTHREADS=
+NO_PREAD=YesPlease

Regards,
Dave
-- 
John David Anglin  dave.anglin@bell.net

^ permalink raw reply related

* Re: [Feature request] An easier way of rebasing if you just want to "force send" a file back to a previous commit
From: brian m. carlson @ 2020-01-12 19:56 UTC (permalink / raw)
  To: 1234dev; +Cc: git@vger.kernel.org
In-Reply-To: <jvRjyPq1IXAbIqfIOfEu2KxNKCMq9ktnAlVF9jGrccIvlPt22V62Ic8j0dHvLDOS31YrHZ2_t8ldgUTgJQHGdsMiYhnpYJJmOlJQFwiif_8=@protonmail.com>

[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]

On 2020-01-12 at 19:42:46, 1234dev wrote:
> Hello!
> 
> I'm pretty new to all of this, but I was wondering. Is there an easier way of rebasing if you just want to force send a file back to a previous commit? Rebasing can get quite tiresome in the long run. It's like 7 steps, and that's without the merge conflicts someone with my luck is guaranteed to run into.
> 
> For instance, say I've made changes to a file. Those changes are too tiny and insignificant to make a new commit out of - they actually ought to be part of a commit I made last night.
> 
> If there just was a way to cheat :) I'm aware it would rewrite my entire history but that's okay, I haven't shared my repo with anybody yet. Maybe something along the lines of "git rebase --off-she-goes <file> <hash>"?
> 
> As opposed to "git stash && git rebase --interactive '<hash>^' && <change pick => edit> && <apply changes manually> && git add <file> && git commit --amend && git rebase --continue && git stash pop && <merge conflict that requires manual intervention> && git rebase --continue && git stash pop && <still a conflict> && rm <file> && git checkout <file> && <repeat the whole process> && <still a conflict> && <go to IRC and ask for help>

The way I usually handle this is something like the following, although
I have some helper aliases that wrap this:

  git add <file>
  git commit --fixup <hash>
  git stash # if necessary
  GIT_SEQUENCE_EDITOR=true git rebase -i --autosquash

That does use the interactive machinery to apply the fixup commit, but
it also avoids prompting you to edit the interactive TODO list.  It
doesn't avoid the merge conflicts which can occur, but it is (IMO) the
easiest way to go about it.

If I'd like to edit the message, I use "git commit --squash" to add the
comments I'd like to add and I'm only prompted to squash together those
messages.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply

* Re: [Feature request] An easier way of rebasing if you just want to "force send" a file back to a previous commit
From: 1234dev @ 2020-01-12 20:41 UTC (permalink / raw)
  To: brian m. carlson, git@vger.kernel.org
In-Reply-To: <20200112195646.GQ6570@camp.crustytoothpaste.net>

Very cool :-)

Thank you so much for making my life easier! Have a good day :-)

Best regards,
Jonathan


Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, January 12, 2020 7:56 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

> On 2020-01-12 at 19:42:46, 1234dev wrote:
>
> > Hello!
> > I'm pretty new to all of this, but I was wondering. Is there an easier way of rebasing if you just want to force send a file back to a previous commit? Rebasing can get quite tiresome in the long run. It's like 7 steps, and that's without the merge conflicts someone with my luck is guaranteed to run into.
> > For instance, say I've made changes to a file. Those changes are too tiny and insignificant to make a new commit out of - they actually ought to be part of a commit I made last night.
> > If there just was a way to cheat :) I'm aware it would rewrite my entire history but that's okay, I haven't shared my repo with anybody yet. Maybe something along the lines of "git rebase --off-she-goes <file> <hash>"?
> > As opposed to "git stash && git rebase --interactive '<hash>^' && <change pick => edit> && <apply changes manually> && git add <file> && git commit --amend && git rebase --continue && git stash pop && <merge conflict that requires manual intervention> && git rebase --continue && git stash pop && <still a conflict> && rm <file> && git checkout <file> && <repeat the whole process> && <still a conflict> && <go to IRC and ask for help>
>
> The way I usually handle this is something like the following, although
> I have some helper aliases that wrap this:
>
> git add <file>
> git commit --fixup <hash>
>
> git stash # if necessary
> GIT_SEQUENCE_EDITOR=true git rebase -i --autosquash
>
> That does use the interactive machinery to apply the fixup commit, but
> it also avoids prompting you to edit the interactive TODO list. It
> doesn't avoid the merge conflicts which can occur, but it is (IMO) the
> easiest way to go about it.
>
> If I'd like to edit the message, I use "git commit --squash" to add the
> comments I'd like to add and I'm only prompted to squash together those
> messages.
>
> --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204



^ permalink raw reply

* Re: [Feature request] An easier way of rebasing if you just want to "force send" a file back to a previous commit
From: Taylor Blau @ 2020-01-12 20:45 UTC (permalink / raw)
  To: brian m. carlson, 1234dev, git@vger.kernel.org
In-Reply-To: <20200112195646.GQ6570@camp.crustytoothpaste.net>

On Sun, Jan 12, 2020 at 07:56:46PM +0000, brian m. carlson wrote:
> On 2020-01-12 at 19:42:46, 1234dev wrote:
> > Hello!
> >
> > I'm pretty new to all of this, but I was wondering. Is there an easier way of rebasing if you just want to force send a file back to a previous commit? Rebasing can get quite tiresome in the long run. It's like 7 steps, and that's without the merge conflicts someone with my luck is guaranteed to run into.
> >
> > For instance, say I've made changes to a file. Those changes are too tiny and insignificant to make a new commit out of - they actually ought to be part of a commit I made last night.
> >
> > If there just was a way to cheat :) I'm aware it would rewrite my entire history but that's okay, I haven't shared my repo with anybody yet. Maybe something along the lines of "git rebase --off-she-goes <file> <hash>"?
> >
> > As opposed to "git stash && git rebase --interactive '<hash>^' && <change pick => edit> && <apply changes manually> && git add <file> && git commit --amend && git rebase --continue && git stash pop && <merge conflict that requires manual intervention> && git rebase --continue && git stash pop && <still a conflict> && rm <file> && git checkout <file> && <repeat the whole process> && <still a conflict> && <go to IRC and ask for help>
>
> The way I usually handle this is something like the following, although
> I have some helper aliases that wrap this:
>
>   git add <file>
>   git commit --fixup <hash>
>   git stash # if necessary
>   GIT_SEQUENCE_EDITOR=true git rebase -i --autosquash
>
> That does use the interactive machinery to apply the fixup commit, but
> it also avoids prompting you to edit the interactive TODO list.  It
> doesn't avoid the merge conflicts which can occur, but it is (IMO) the
> easiest way to go about it.

I couldn't quite tell one way or the other, but I think that the
original poster was asking about the case in which one wants to move
some hunks out of one commit and into an earlier one.

I usually go about this with something like:

  h="$(git rev-parse HEAD)"
  git reset HEAD^
  git add <file[s]> # re-stage the files that you want to move "up"
  git commit --fixup <hash>
  git add --all .
  git commit -C "$h" # "re-apply" the commit that you were moving out of
  GIT_SEQUENCE_EDITOR=true git rebase -i --autosquash

> If I'd like to edit the message, I use "git commit --squash" to add the
> comments I'd like to add and I'm only prompted to squash together those
> messages.

My preference is usually to allow 'git rebase -i' to open my "$EDITOR"
and change the todo list to "reword" for any commit(s) for which I want
to change the message.

> --
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204

Thanks,
Taylor

^ permalink raw reply

* Re: Problems with ra/rebase-i-more-options - should we revert it?
From: Junio C Hamano @ 2020-01-12 21:12 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Git Mailing List, Johannes Schindelin, Rohit Ashiwal,
	Jonathan Nieder
In-Reply-To: <089637d7-b4b6-f6ba-cce1-29e22ce47521@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 12/01/2020 16:12, Phillip Wood wrote:
>> I'm concerned that there are some bugs in this series and think
>> it may be best to revert it before releasing 2.25.0.

Let's do that.

>> Jonathan
>> Nieder posted a bug report on Friday [1] which I think is caused
>> by this series. While trying to reproduce Jonathan's bug I came
>> up with the test below which fails, but not in the same way.
>
> Doh I forgot to add --committer-date-is-author-date to the rebase
> command line in that test. It passes with that added - how
> embarrassing. However it does appear that it prefixes the date in
> GIT_COMMITTER_DATE with @@ rather than @.
>
> start of the epoch and fails to parse it. The fix for the @@ is
> quite simple, the date we read from the author script already has
> an @ so we don't need to add another.

Yes, that sounds like a minimum and straightforward fix.

In any case, the tip of 'master' (hence the one that would become
the final) is simpler to remedy by just reverting the merge, but
there are a handful of in-flight topics that may have been queued by
forking 'master' after the problematic merge was made (iow, anything
after the fifth batch for this cycle), which I'd have to be a bit
careful when I merge them down, lest they attempt to pull in the bad
topic again.  But that will be something we need to worry about
after the release, not before the final.

Thanks.


[Footnote]

*1* The list of still-in-flight topics that may be contaminated with
    the merge of ra/rebase-i-more-options into 'master' are:

    am/test-pathspec-f-f-error-cases
    am/update-pathspec-f-f-tests
    bc/hash-independent-tests-part-7
    dl/merge-autostash
    ds/graph-horizontal-edges
    en/rebase-backend
    es/bugreport
    es/pathspec-f-f-grep
    hi/gpg-mintrustlevel
    hw/advice-add-nothing
    jn/promote-proto2-to-default
    jn/test-lint-one-shot-export-to-shell-function
    kw/fsmonitor-watchman-racefix
    sg/completion-worktree
    yz/p4-py3

I probably may requeue them by rebasing on top of 2.25 once the
release is done.

^ permalink raw reply

* Re: [PATCH v3 15/15] rebase: change the default backend from "am" to "merge"
From: Junio C Hamano @ 2020-01-12 21:23 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Elijah Newren, Jonathan Nieder, Elijah Newren via GitGitGadget,
	Git Mailing List, Johannes Schindelin, Phillip Wood, Denton Liu,
	Pavel Roskin, Alban Gruin, SZEDER Gábor
In-Reply-To: <052fdedc-2beb-91ab-c5c3-53fb99e64810@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

> Looking through the history the am based rebase has never run the
> post-commit hook as am has its own set of hooks and the scripted
> version used commit-tree.
>
>> But the big question here, is what is correct behavior?  Should rebase
>> call the post-commit hook, or should it skip it?  I haven't any clue
>> what the answer to that is.
>
> It's creating a new commit so I lean towards thinking it should run
> the post-commit hook. As an example I have a post-commit hook that

The reason why "am" did not run "post-commit" is because
"post-commit" was (originally) meant to be for cases where the end
user types "git commit", IOW, the notion of "since we are creating a
new commit object, let's run the post-commit hook" used to be
totally misguided way of thinking.  The hook was "the user _ran_
'commit', so post-commit is run immediately afterwards".

These days, I think most of our tools subscribe to the newer "a
commit got created--have hook the chance to report it" idea, simply
because the original position is untenable---"git merge" may not
want to run the post-commit hook according to the initial design,
and it is fine to make it not to run it when clean automerge was
recorded, but sometimes the end users have to type "git commit" to
conclude a conflicted merge.

So from that point of view, I do not think it is too bad to bring
"am" into the new ear and start running post-commit from it.

The "pre-commit" hook is a different story, though.  From purely
practical point of view, starting to run extra verification that may
veto new commits from getting created only because we changed the
implementation detail for some reason is a disservice to the users.


^ permalink raw reply

* Fwd: Issue with Git Branch
From: Jude Guan Wang @ 2020-01-13  0:01 UTC (permalink / raw)
  To: git
In-Reply-To: <56F9DC91-740F-47C7-9C2A-B6B1EC3A12B6@clicktherapeutics.com>

Hi, all:
	I noticed something weird with my git command. In my local environment I seem to have a branch named `-D`:

	And I don’t remember how I was able to create a branch like that. I tried to do git branch -D/-d this branch but seems not working. I’m assuming the branch infos were kept in .git folder so my question is if there’s anyway I could remove this invalid branch from that folder? My git version is 2.24.0. Thanks for any help in advance.


^ permalink raw reply

* Re: Fwd: Issue with Git Branch
From: Taylor Blau @ 2020-01-13  0:11 UTC (permalink / raw)
  To: Jude Guan Wang; +Cc: git
In-Reply-To: <29742805-4992-47E6-9889-F55F5EFBBFF1@clicktherapeutics.com>

Hi Jude,

On Sun, Jan 12, 2020 at 07:01:04PM -0500, Jude Guan Wang wrote:
> Hi, all:
> 	I noticed something weird with my git command. In my local
> 	environment I seem to have a branch named `-D`:
>
> 	And I don’t remember how I was able to create a branch like that. I
> 	tried to do git branch -D/-d this branch but seems not working. I’m
> 	assuming the branch infos were kept in .git folder so my question is
> 	if there’s anyway I could remove this invalid branch from that
> 	folder? My git version is 2.24.0. Thanks for any help in advance.

You can delete oddly-named branches by passing the end-of-options marker
to 'git branch', like so:

  $ git branch -D --end-of-options -D

(In fact, '--end-of-options' isn't strictly necessary here, and using
'--' as in 'git branch -D -- -D' will work as well).

If you do find a reproducible way to create branches named '-D' or
similar, please do let us know, as these are not intended to be valid
branch names in general.

Thanks,
Taylor

^ permalink raw reply

* Re: Fwd: Issue with Git Branch
From: SZEDER Gábor @ 2020-01-13  0:42 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Jude Guan Wang, git
In-Reply-To: <20200113001143.GA13099@syl.local>

On Sun, Jan 12, 2020 at 04:11:43PM -0800, Taylor Blau wrote:
> On Sun, Jan 12, 2020 at 07:01:04PM -0500, Jude Guan Wang wrote:
> > 	I noticed something weird with my git command. In my local
> > 	environment I seem to have a branch named `-D`:
> >
> > 	And I don’t remember how I was able to create a branch like that.

> If you do find a reproducible way to create branches named '-D' or
> similar, please do let us know, as these are not intended to be valid
> branch names in general.

  $ git update-ref refs/heads/-D master
  $ git branch |head -n1
    -D


^ permalink raw reply

* Re: Problems with ra/rebase-i-more-options - should we revert it?
From: Junio C Hamano @ 2020-01-13  0:43 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Git Mailing List, Johannes Schindelin, Rohit Ashiwal,
	Jonathan Nieder
In-Reply-To: <xmqqeew4l6qf.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> On 12/01/2020 16:12, Phillip Wood wrote:
>>> I'm concerned that there are some bugs in this series and think
>>> it may be best to revert it before releasing 2.25.0.
>
> Let's do that.
> ...
>>> J
>
> In any case, the tip of 'master' (hence the one that would become
> the final) is simpler to remedy by just reverting the merge, but
> there are a handful of in-flight topics that may have been queued by
> forking 'master' after the problematic merge was made (iow, anything
> after the fifth batch for this cycle), which I'd have to be a bit
> careful when I merge them down, lest they attempt to pull in the bad
> topic again.  But that will be something we need to worry about
> after the release, not before the final.

I will push out what I wish to be able to tag as the final [*1*]
shortly but without actually tagging, so that it can get a bit wider
exposure than just the usual "Gitster tested locally and then did
let Travis try them" testing.

Thanks.


[Reference]

*1* The tip of 'master' as of this writing is v2.25.0-rc2-24-gb4615e40a8


^ permalink raw reply

* Re: Fwd: Issue with Git Branch
From: Taylor Blau @ 2020-01-13  0:48 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Taylor Blau, Jude Guan Wang, git
In-Reply-To: <20200113004235.GI32750@szeder.dev>

On Mon, Jan 13, 2020 at 01:42:35AM +0100, SZEDER Gábor wrote:
> On Sun, Jan 12, 2020 at 04:11:43PM -0800, Taylor Blau wrote:
> > On Sun, Jan 12, 2020 at 07:01:04PM -0500, Jude Guan Wang wrote:
> > > 	I noticed something weird with my git command. In my local
> > > 	environment I seem to have a branch named `-D`:
> > >
> > > 	And I don’t remember how I was able to create a branch like that.
>
> > If you do find a reproducible way to create branches named '-D' or
> > similar, please do let us know, as these are not intended to be valid
> > branch names in general.
>
>   $ git update-ref refs/heads/-D master
>   $ git branch |head -n1
>     -D

I was assuming that Jude had gotten the ref to appear by using 'git
branch' alone, i.e., without the help of 'git update-ref' or 'cp
.git/refs/heads/{master,-D}'.

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase
From: Eric Sunshine @ 2020-01-13  1:59 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Marc-André Lureau, Git List
In-Reply-To: <20200112121402.GH32750@szeder.dev>

On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > Taking a deeper look at the code, I'm wondering it would make more
> > sense to call wt_status_get_state(), which handles 'rebase' and
> > 'bisect'. Is there a reason that you limited this check to only
> > 'rebase'?
>
> What branch name does wt_status_get_state() return while bisecting?
> The branch where I started from?  Because that's what 'git status'
> shows:
> But am I really on that branch?  Does it really makes sense to edit
> the description of 'mybranch' by default while bisecting through an
> old revision range?  I do not think so.

It's not clear what downside you are pointing out; i.e. why would it
be a bad thing to be able to set the branch description even while
bisecting -- especially since `git status` affirms that it knows the
branch?

> > Looking at the code itself (rather than consulting only the patch), I
> > see that there are a couple more early returns leaking 'branch_name',
> > so they need to be handled, as well.
>
> 'git branch --edit-description' is a one-shot operation: it allows to
> edit only one branch description per invocation, and then the process
> exits right away, whether the operation was successful or some error
> occurred.

It is one-shot, but the existing `--edit-description` code already
cleans up after itself by releasing resources it allocated (as do
other one-shot parts of cmd_branch()), so it would be odd and
inconsistent for this new code to not clean up after itself, as well
(or, more accurately, to only clean up after itself in some branches
but not others).

> I'm not sure free()ing 'branch_name' is worth the effort
> (and even if it does, I think it should be a separate preparatory
> patch).

A separate preparatory patch doesn't make sense in this case since
'branch_name' becomes "freeable" with this patch itself (prior to
that, it was `const char *`).

Anyhow, a different approach was later proposed[1] which eliminates
some of the ugliness.

[1]: https://lore.kernel.org/git/20200112101735.GA19676@flurp.local/

^ permalink raw reply

* Dropbox do not sync particular files after using git push
From: On Luk @ 2020-01-13  2:56 UTC (permalink / raw)
  To: git
In-Reply-To: <9A165840-280C-43C4-B92B-3349E07AC0B1@webssup.com>

Hi Sir/Madam,

I tried to report this issue to dropbox technical team, but they replied that the issue was not caused by Dropbox, so I try to report to your team as a bug report below:

Everytime I used git to push commit to my remote repo located in my dropbox, some files stuck in syncing status and can’t be able to sync to dropbox. To get those files be synced, I need to do one of the follow steps everytime by manually:

1.	Drag and drop back the file to dropbox using the finder.
OR
2.	Click Pause and resume syncing in dropbox desktop manager.
OR
3.	Restart Dropbox.

To make it clear, I prepared a screen recording to show the issue that I am facing.

www.dropbox.com/s/ivn7qytk0u67v6n/Screen%20Recording%202020-01-03%20at%2012.21.21%20PM.mov?dl=0

This issue only appear after I switch to use my new computer running as macOS 10.15, everything works fine in my old computer running as macOS 10.13.6.

p.s: git version 2.24.1

BRs,

On
-- 










The information in this electronic mail ("e-mail") message is for 
the use of the named recipient(s) only and may be confidential. The 
information may be protected by privilege, work product immunity or other 
applicable law. If you are not the intended recipient(s), the retention, 
dissemination, distribution or copying of this e-mail message is strictly 
prohibited. If you receive this e-mail message in error, please notify me 
immediately by replying to the message and thereafter, delete all copies on 
your system and destroy any hard copies. 

^ permalink raw reply

* Re: [PATCH v2 1/9] built-in add -p: support interactive.diffFilter
From: Johannes Schindelin @ 2020-01-13  6:47 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano
In-Reply-To: <20200107225749.GD32750@szeder.dev>

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hi Gábor,

On Tue, 7 Jan 2020, SZEDER Gábor wrote:

> On Wed, Dec 25, 2019 at 11:56:52AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > The Perl version supports post-processing the colored diff (that is
> > generated in addition to the uncolored diff, intended to offer a
> > prettier user experience) by a command configured via that config
> > setting, and now the built-in version does that, too.
>
> So this patch makes the test 'detect bogus diffFilter output' in
> 't3701-add-interactive.sh' succeed with the builtin interactive add,
> but I stumbled upon a test failure caused by SIGPIPE in an
> experimental Travis CI s390x build:
>
>   expecting success of 3701.49 'detect bogus diffFilter output':
>           git reset --hard &&
>
>           echo content >test &&
>           test_config interactive.diffFilter "echo too-short" &&
>           printf y >y &&
>           test_must_fail force_color git add -p <y
>
>   + git reset --hard
>   HEAD is now at 6ee5ee5 test
>   + echo content
>   + test_config interactive.diffFilter echo too-short
>   + printf y
>   + test_must_fail force_color git add -p
>   test_must_fail: died by signal 13: force_color git add -p
>   error: last command exited with $?=1
>
> Turns out it's a general issue, and
>
>   GIT_TEST_ADD_I_USE_BUILTIN=1 ./t3701-add-interactive.sh -r 39,49 --stress
>
> fails within 10 seconds on my Linux box, whereas the scripted 'add -p'
> managed to survive a couple hundred repetitions.

You're right, of course. And I had let that slip for too long, as I saw it
sporadically happen in the Azure Pipeline, too.

This took quite a while to figure out, and I won't claim that I understand
_all_ the details: I _think_ that `stdin` being so short "breaks the pipe"
and interferes with `add -p`'s normal operation, so I needed to explicitly
use the `sigchain` feature to ignore `SIGPIPE` during `add -p`'s main
loop.

Thanks,
Dscho

^ permalink raw reply

* [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

As noticed by Gábor Szeder, if we want to run `git add -p` with
redirected input through `test_must_fail` in the test suite, we must
expect that a SIGPIPE can happen due to `stdin` coming to its end.

The appropriate action here is to ignore that signal and treat it as a
regular end-of-file, otherwise the test will fail. In preparation for
such a test, introduce precisely this handling of SIGPIPE into the
built-in version of `git add -p`.

For good measure, teach the built-in `git add -i` the same trick: it
_also_ runs a loop waiting for input, and can receive a SIGPIPE just the
same (and wants to treat it as end-of-file, too).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 3 +++
 add-patch.c       | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/add-interactive.c b/add-interactive.c
index a5bb14f2f4..3ff8400ea4 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -9,6 +9,7 @@
 #include "lockfile.h"
 #include "dir.h"
 #include "run-command.h"
+#include "sigchain.h"
 
 static void init_color(struct repository *r, struct add_i_state *s,
 		       const char *slot_name, char *dst,
@@ -1097,6 +1098,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 			->util = util;
 	}
 
+	sigchain_push(SIGPIPE, SIG_IGN);
 	init_add_i_state(&s, r);
 
 	/*
@@ -1149,6 +1151,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	strbuf_release(&print_file_item_data.worktree);
 	strbuf_release(&header);
 	prefix_item_list_clear(&commands);
+	sigchain_pop(SIGPIPE);
 
 	return res;
 }
diff --git a/add-patch.c b/add-patch.c
index 46c6c183d5..9a3beed72e 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -6,6 +6,7 @@
 #include "pathspec.h"
 #include "color.h"
 #include "diff.h"
+#include "sigchain.h"
 
 enum prompt_mode_type {
 	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,
@@ -1578,6 +1579,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	};
 	size_t i, binary_count = 0;
 
+	sigchain_push(SIGPIPE, SIG_IGN);
 	init_add_i_state(&s.s, r);
 
 	if (mode == ADD_P_STASH)
@@ -1612,6 +1614,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	    parse_diff(&s, ps) < 0) {
 		strbuf_release(&s.plain);
 		strbuf_release(&s.colored);
+		sigchain_pop(SIGPIPE);
 		return -1;
 	}
 
@@ -1630,5 +1633,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	strbuf_release(&s.buf);
 	strbuf_release(&s.plain);
 	strbuf_release(&s.colored);
+	sigchain_pop(SIGPIPE);
 	return 0;
 }
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 02/10] built-in add -p: support interactive.diffFilter
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The Perl version supports post-processing the colored diff (that is
generated in addition to the uncolored diff, intended to offer a
prettier user experience) by a command configured via that config
setting, and now the built-in version does that, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 12 ++++++++++++
 add-interactive.h |  3 +++
 add-patch.c       | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/add-interactive.c b/add-interactive.c
index 3ff8400ea4..b36e5d97d8 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -53,6 +53,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)
 		diff_get_color(s->use_color, DIFF_FILE_OLD));
 	init_color(r, s, "new", s->file_new_color,
 		diff_get_color(s->use_color, DIFF_FILE_NEW));
+
+	FREE_AND_NULL(s->interactive_diff_filter);
+	git_config_get_string("interactive.difffilter",
+			      &s->interactive_diff_filter);
+}
+
+void clear_add_i_state(struct add_i_state *s)
+{
+	FREE_AND_NULL(s->interactive_diff_filter);
+	memset(s, 0, sizeof(*s));
+	s->use_color = -1;
 }
 
 /*
@@ -1151,6 +1162,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	strbuf_release(&print_file_item_data.worktree);
 	strbuf_release(&header);
 	prefix_item_list_clear(&commands);
+	clear_add_i_state(&s);
 	sigchain_pop(SIGPIPE);
 
 	return res;
diff --git a/add-interactive.h b/add-interactive.h
index b2f23479c5..46c73867ad 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -15,9 +15,12 @@ struct add_i_state {
 	char context_color[COLOR_MAXLEN];
 	char file_old_color[COLOR_MAXLEN];
 	char file_new_color[COLOR_MAXLEN];
+
+	char *interactive_diff_filter;
 };
 
 void init_add_i_state(struct add_i_state *s, struct repository *r);
+void clear_add_i_state(struct add_i_state *s);
 
 struct repository;
 struct pathspec;
diff --git a/add-patch.c b/add-patch.c
index 9a3beed72e..7d6015229c 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -399,6 +399,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 
 	if (want_color_fd(1, -1)) {
 		struct child_process colored_cp = CHILD_PROCESS_INIT;
+		const char *diff_filter = s->s.interactive_diff_filter;
 
 		setup_child_process(s, &colored_cp, NULL);
 		xsnprintf((char *)args.argv[color_arg_index], 8, "--color");
@@ -408,6 +409,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 		argv_array_clear(&args);
 		if (res)
 			return error(_("could not parse colored diff"));
+
+		if (diff_filter) {
+			struct child_process filter_cp = CHILD_PROCESS_INIT;
+
+			setup_child_process(s, &filter_cp,
+					    diff_filter, NULL);
+			filter_cp.git_cmd = 0;
+			filter_cp.use_shell = 1;
+			strbuf_reset(&s->buf);
+			if (pipe_command(&filter_cp,
+					 colored->buf, colored->len,
+					 &s->buf, colored->len,
+					 NULL, 0) < 0)
+				return error(_("failed to run '%s'"),
+					     diff_filter);
+			strbuf_swap(colored, &s->buf);
+		}
+
 		strbuf_complete_line(colored);
 		colored_p = colored->buf;
 		colored_pend = colored_p + colored->len;
@@ -532,6 +551,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 						   colored_pend - colored_p);
 			if (colored_eol)
 				colored_p = colored_eol + 1;
+			else if (p != pend)
+				/* colored shorter than non-colored? */
+				goto mismatched_output;
 			else
 				colored_p = colored_pend;
 
@@ -556,6 +578,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 		 */
 		hunk->splittable_into++;
 
+	/* non-colored shorter than colored? */
+	if (colored_p != colored_pend) {
+mismatched_output:
+		error(_("mismatched output from interactive.diffFilter"));
+		advise(_("Your filter must maintain a one-to-one correspondence\n"
+			 "between its input and output lines."));
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1614,6 +1645,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	    parse_diff(&s, ps) < 0) {
 		strbuf_release(&s.plain);
 		strbuf_release(&s.colored);
+		clear_add_i_state(&s.s);
 		sigchain_pop(SIGPIPE);
 		return -1;
 	}
@@ -1633,6 +1665,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	strbuf_release(&s.buf);
 	strbuf_release(&s.plain);
 	strbuf_release(&s.colored);
+	clear_add_i_state(&s.s);
 	sigchain_pop(SIGPIPE);
 	return 0;
 }
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 03/10] built-in add -p: handle diff.algorithm
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The Perl version of `git add -p` reads the config setting
`diff.algorithm` and if set, uses it to generate the diff using the
specified algorithm.

This patch ports that functionality to the C version.

Note: just like `git-add--interactive.perl`, we do _not_ respect this
config setting in `git add -i`'s `diff` command, but _only_ in the
`patch` command.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c | 5 +++++
 add-interactive.h | 2 +-
 add-patch.c       | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/add-interactive.c b/add-interactive.c
index b36e5d97d8..e3cc30ad24 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -57,11 +57,16 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)
 	FREE_AND_NULL(s->interactive_diff_filter);
 	git_config_get_string("interactive.difffilter",
 			      &s->interactive_diff_filter);
+
+	FREE_AND_NULL(s->interactive_diff_algorithm);
+	git_config_get_string("diff.algorithm",
+			      &s->interactive_diff_algorithm);
 }
 
 void clear_add_i_state(struct add_i_state *s)
 {
 	FREE_AND_NULL(s->interactive_diff_filter);
+	FREE_AND_NULL(s->interactive_diff_algorithm);
 	memset(s, 0, sizeof(*s));
 	s->use_color = -1;
 }
diff --git a/add-interactive.h b/add-interactive.h
index 46c73867ad..923efaf527 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -16,7 +16,7 @@ struct add_i_state {
 	char file_old_color[COLOR_MAXLEN];
 	char file_new_color[COLOR_MAXLEN];
 
-	char *interactive_diff_filter;
+	char *interactive_diff_filter, *interactive_diff_algorithm;
 };
 
 void init_add_i_state(struct add_i_state *s, struct repository *r);
diff --git a/add-patch.c b/add-patch.c
index 7d6015229c..736bcb4aa7 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -361,6 +361,7 @@ static int is_octal(const char *p, size_t len)
 static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 {
 	struct argv_array args = ARGV_ARRAY_INIT;
+	const char *diff_algorithm = s->s.interactive_diff_algorithm;
 	struct strbuf *plain = &s->plain, *colored = NULL;
 	struct child_process cp = CHILD_PROCESS_INIT;
 	char *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\0';
@@ -370,6 +371,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 	int res;
 
 	argv_array_pushv(&args, s->mode->diff_cmd);
+	if (diff_algorithm)
+		argv_array_pushf(&args, "--diff-algorithm=%s", diff_algorithm);
 	if (s->revision) {
 		struct object_id oid;
 		argv_array_push(&args,
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 00/10] built-in add -p: add support for the same config settings as the Perl version
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <pull.175.v2.git.1577275020.gitgitgadget@gmail.com>

This is the final leg of the journey to a fully built-in git add: the git
add -i and git add -p modes were re-implemented in C, but they lacked
support for a couple of config settings.

The one that sticks out most is the interactive.singleKey setting: it was
particularly hard to get to work, especially on Windows.

It also seems to be the setting that is incomplete already in the Perl
version of the interactive add command: while the name of the config setting
suggests that it applies to all of the interactive add, including the main
loop of git add --interactive and to the file selections in that command, it
does not. Only the git add --patch mode respects that setting.

As it is outside the purpose of the conversion of git-add--interactive.perl 
to C, we will leave that loose end for some future date.

Changes since v2:

 * Fixed the SIGPIPE issue pointed out by Gábor Szeder.

Changes since v1:

 * Fixed the commit message where a copy/paste fail made it talk about
   another GIT_TEST_* variable than the GIT_TEST_ADD_I_USE_BUILTIN one.

Johannes Schindelin (10):
  built-in add -i/-p: treat SIGPIPE as EOF
  built-in add -p: support interactive.diffFilter
  built-in add -p: handle diff.algorithm
  terminal: make the code of disable_echo() reusable
  terminal: accommodate Git for Windows' default terminal
  terminal: add a new function to read a single keystroke
  built-in add -p: respect the `interactive.singlekey` config setting
  built-in add -p: handle Escape sequences in interactive.singlekey mode
  built-in add -p: handle Escape sequences more efficiently
  ci: include the built-in `git add -i` in the `linux-gcc` job

 add-interactive.c         |  22 ++++
 add-interactive.h         |   4 +
 add-patch.c               |  61 +++++++++-
 ci/run-build-and-tests.sh |   1 +
 compat/terminal.c         | 249 +++++++++++++++++++++++++++++++++++++-
 compat/terminal.h         |   3 +
 6 files changed, 332 insertions(+), 8 deletions(-)


base-commit: c480eeb574e649a19f27dc09a994e45f9b2c2622
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-175%2Fdscho%2Fadd-p-in-c-config-settings-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-175/dscho/add-p-in-c-config-settings-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/175

Range-diff vs v2:

  -:  ---------- >  1:  5e258a8d2b built-in add -i/-p: treat SIGPIPE as EOF
  1:  f45ff08bd0 !  2:  2a5951ecfe built-in add -p: support interactive.diffFilter
     @@ -35,9 +35,9 @@
       	strbuf_release(&header);
       	prefix_item_list_clear(&commands);
      +	clear_add_i_state(&s);
     + 	sigchain_pop(SIGPIPE);
       
       	return res;
     - }
      
       diff --git a/add-interactive.h b/add-interactive.h
       --- a/add-interactive.h
     @@ -123,13 +123,14 @@
       		strbuf_release(&s.plain);
       		strbuf_release(&s.colored);
      +		clear_add_i_state(&s.s);
     + 		sigchain_pop(SIGPIPE);
       		return -1;
       	}
     - 
      @@
       	strbuf_release(&s.buf);
       	strbuf_release(&s.plain);
       	strbuf_release(&s.colored);
      +	clear_add_i_state(&s.s);
     + 	sigchain_pop(SIGPIPE);
       	return 0;
       }
  2:  e9c4a13cbf =  3:  a2bce01818 built-in add -p: handle diff.algorithm
  3:  e643554dba =  4:  be40a37c0c terminal: make the code of disable_echo() reusable
  4:  bd2306c5d5 =  5:  233f23791c terminal: accommodate Git for Windows' default terminal
  5:  190fb4f5e9 =  6:  74593b5115 terminal: add a new function to read a single keystroke
  6:  167dfa37dd !  7:  197fe1e14a built-in add -p: respect the `interactive.singlekey` config setting
     @@ -48,9 +48,9 @@
       --- a/add-patch.c
       +++ b/add-patch.c
      @@
     - #include "pathspec.h"
       #include "color.h"
       #include "diff.h"
     + #include "sigchain.h"
      +#include "compat/terminal.h"
       
       enum prompt_mode_type {
  7:  32067bebe8 =  8:  9ab381d539 built-in add -p: handle Escape sequences in interactive.singlekey mode
  8:  703719ffce =  9:  bdb6268b8b built-in add -p: handle Escape sequences more efficiently
  9:  23a3a47b01 = 10:  c4195969a6 ci: include the built-in `git add -i` in the `linux-gcc` job

-- 
gitgitgadget

^ permalink raw reply

* [PATCH v3 05/10] terminal: accommodate Git for Windows' default terminal
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Git for Windows' Git Bash runs in MinTTY by default, which does not have
a Win32 Console instance, but uses MSYS2 pseudo terminals instead.

This is a problem, as Git for Windows does not want to use the MSYS2
emulation layer for Git itself, and therefore has no direct way to
interact with that pseudo terminal.

As a workaround, use the `stty` utility (which is included in Git for
Windows, and which *is* an MSYS2 program, so it knows how to deal with
the pseudo terminal).

Note: If Git runs in a regular CMD or PowerShell window, there *is* a
regular Win32 Console to work with. This is not a problem for the MSYS2
`stty`: it copes with this scenario just fine.

Also note that we introduce support for more bits than would be
necessary for a mere `disable_echo()` here, in preparation for the
upcoming `enable_non_canonical()` function.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/terminal.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/compat/terminal.c b/compat/terminal.c
index 1fb40b3a0a..16e9949da1 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -2,6 +2,8 @@
 #include "compat/terminal.h"
 #include "sigchain.h"
 #include "strbuf.h"
+#include "run-command.h"
+#include "string-list.h"
 
 #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)
 
@@ -64,11 +66,28 @@ static int disable_echo(void)
 #define OUTPUT_PATH "CONOUT$"
 #define FORCE_TEXT "t"
 
+static int use_stty = 1;
+static struct string_list stty_restore = STRING_LIST_INIT_DUP;
 static HANDLE hconin = INVALID_HANDLE_VALUE;
 static DWORD cmode;
 
 static void restore_term(void)
 {
+	if (use_stty) {
+		int i;
+		struct child_process cp = CHILD_PROCESS_INIT;
+
+		if (stty_restore.nr == 0)
+			return;
+
+		argv_array_push(&cp.args, "stty");
+		for (i = 0; i < stty_restore.nr; i++)
+			argv_array_push(&cp.args, stty_restore.items[i].string);
+		run_command(&cp);
+		string_list_clear(&stty_restore, 0);
+		return;
+	}
+
 	if (hconin == INVALID_HANDLE_VALUE)
 		return;
 
@@ -79,6 +98,37 @@ static void restore_term(void)
 
 static int disable_bits(DWORD bits)
 {
+	if (use_stty) {
+		struct child_process cp = CHILD_PROCESS_INIT;
+
+		argv_array_push(&cp.args, "stty");
+
+		if (bits & ENABLE_LINE_INPUT) {
+			string_list_append(&stty_restore, "icanon");
+			argv_array_push(&cp.args, "-icanon");
+		}
+
+		if (bits & ENABLE_ECHO_INPUT) {
+			string_list_append(&stty_restore, "echo");
+			argv_array_push(&cp.args, "-echo");
+		}
+
+		if (bits & ENABLE_PROCESSED_INPUT) {
+			string_list_append(&stty_restore, "-ignbrk");
+			string_list_append(&stty_restore, "intr");
+			string_list_append(&stty_restore, "^c");
+			argv_array_push(&cp.args, "ignbrk");
+			argv_array_push(&cp.args, "intr");
+			argv_array_push(&cp.args, "");
+		}
+
+		if (run_command(&cp) == 0)
+			return 0;
+
+		/* `stty` could not be executed; access the Console directly */
+		use_stty = 0;
+	}
+
 	hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE,
 	    FILE_SHARE_READ, NULL, OPEN_EXISTING,
 	    FILE_ATTRIBUTE_NORMAL, NULL);
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 04/10] terminal: make the code of disable_echo() reusable
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

We are about to introduce the function `enable_non_canonical()`, which
shares almost the complete code with `disable_echo()`.

Let's prepare for that, by refactoring out that shared code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/terminal.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/compat/terminal.c b/compat/terminal.c
index fa13ee672d..1fb40b3a0a 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -32,7 +32,7 @@ static void restore_term(void)
 	term_fd = -1;
 }
 
-static int disable_echo(void)
+static int disable_bits(tcflag_t bits)
 {
 	struct termios t;
 
@@ -43,7 +43,7 @@ static int disable_echo(void)
 	old_term = t;
 	sigchain_push_common(restore_term_on_signal);
 
-	t.c_lflag &= ~ECHO;
+	t.c_lflag &= ~bits;
 	if (!tcsetattr(term_fd, TCSAFLUSH, &t))
 		return 0;
 
@@ -53,6 +53,11 @@ static int disable_echo(void)
 	return -1;
 }
 
+static int disable_echo(void)
+{
+	return disable_bits(ECHO);
+}
+
 #elif defined(GIT_WINDOWS_NATIVE)
 
 #define INPUT_PATH "CONIN$"
@@ -72,7 +77,7 @@ static void restore_term(void)
 	hconin = INVALID_HANDLE_VALUE;
 }
 
-static int disable_echo(void)
+static int disable_bits(DWORD bits)
 {
 	hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE,
 	    FILE_SHARE_READ, NULL, OPEN_EXISTING,
@@ -82,7 +87,7 @@ static int disable_echo(void)
 
 	GetConsoleMode(hconin, &cmode);
 	sigchain_push_common(restore_term_on_signal);
-	if (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {
+	if (!SetConsoleMode(hconin, cmode & ~bits)) {
 		CloseHandle(hconin);
 		hconin = INVALID_HANDLE_VALUE;
 		return -1;
@@ -91,6 +96,12 @@ static int disable_echo(void)
 	return 0;
 }
 
+static int disable_echo(void)
+{
+	return disable_bits(ENABLE_ECHO_INPUT);
+}
+
+
 #endif
 
 #ifndef FORCE_TEXT
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 06/10] terminal: add a new function to read a single keystroke
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Typically, input on the command-line is line-based. It is actually not
really easy to get single characters (or better put: keystrokes).

We provide two implementations here:

- One that handles `/dev/tty` based systems as well as native Windows.
  The former uses the `tcsetattr()` function to put the terminal into
  "raw mode", which allows us to read individual keystrokes, one by one.
  The latter uses `stty.exe` to do the same, falling back to direct
  Win32 Console access.

  Thanks to the refactoring leading up to this commit, this is a single
  function, with the platform-specific details hidden away in
  conditionally-compiled code blocks.

- A fall-back which simply punts and reads back an entire line.

Note that the function writes the keystroke into an `strbuf` rather than
a `char`, in preparation for reading Escape sequences (e.g. when the
user hit an arrow key). This is also required for UTF-8 sequences in
case the keystroke corresponds to a non-ASCII letter.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/terminal.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 compat/terminal.h |  3 +++
 2 files changed, 58 insertions(+)

diff --git a/compat/terminal.c b/compat/terminal.c
index 16e9949da1..1b2564042a 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -60,6 +60,11 @@ static int disable_echo(void)
 	return disable_bits(ECHO);
 }
 
+static int enable_non_canonical(void)
+{
+	return disable_bits(ICANON | ECHO);
+}
+
 #elif defined(GIT_WINDOWS_NATIVE)
 
 #define INPUT_PATH "CONIN$"
@@ -151,6 +156,10 @@ static int disable_echo(void)
 	return disable_bits(ENABLE_ECHO_INPUT);
 }
 
+static int enable_non_canonical(void)
+{
+	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);
+}
 
 #endif
 
@@ -198,6 +207,33 @@ char *git_terminal_prompt(const char *prompt, int echo)
 	return buf.buf;
 }
 
+int read_key_without_echo(struct strbuf *buf)
+{
+	static int warning_displayed;
+	int ch;
+
+	if (warning_displayed || enable_non_canonical() < 0) {
+		if (!warning_displayed) {
+			warning("reading single keystrokes not supported on "
+				"this platform; reading line instead");
+			warning_displayed = 1;
+		}
+
+		return strbuf_getline(buf, stdin);
+	}
+
+	strbuf_reset(buf);
+	ch = getchar();
+	if (ch == EOF) {
+		restore_term();
+		return EOF;
+	}
+
+	strbuf_addch(buf, ch);
+	restore_term();
+	return 0;
+}
+
 #else
 
 char *git_terminal_prompt(const char *prompt, int echo)
@@ -205,4 +241,23 @@ char *git_terminal_prompt(const char *prompt, int echo)
 	return getpass(prompt);
 }
 
+int read_key_without_echo(struct strbuf *buf)
+{
+	static int warning_displayed;
+	const char *res;
+
+	if (!warning_displayed) {
+		warning("reading single keystrokes not supported on this "
+			"platform; reading line instead");
+		warning_displayed = 1;
+	}
+
+	res = getpass("");
+	strbuf_reset(buf);
+	if (!res)
+		return EOF;
+	strbuf_addstr(buf, res);
+	return 0;
+}
+
 #endif
diff --git a/compat/terminal.h b/compat/terminal.h
index 97db7cd69d..a9d52b8464 100644
--- a/compat/terminal.h
+++ b/compat/terminal.h
@@ -3,4 +3,7 @@
 
 char *git_terminal_prompt(const char *prompt, int echo);
 
+/* Read a single keystroke, without echoing it to the terminal */
+int read_key_without_echo(struct strbuf *buf);
+
 #endif /* COMPAT_TERMINAL_H */
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 07/10] built-in add -p: respect the `interactive.singlekey` config setting
From: Johannes Schindelin via GitGitGadget @ 2020-01-13  8:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.175.v3.git.1578904171.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The Perl version of `git add -p` supports this config setting to allow
users to input commands via single characters (as opposed to having to
press the <Enter> key afterwards).

This is an opt-in feature because it requires Perl packages
(Term::ReadKey and Term::Cap, where it tries to handle an absence of the
latter package gracefully) to work. Note that at least on Ubuntu, that
Perl package is not installed by default (it needs to be installed via
`sudo apt-get install libterm-readkey-perl`), so this feature is
probably not used a whole lot.

In C, we obviously do not have these packages available, but we just
introduced `read_single_keystroke()` that is similar to what
Term::ReadKey provides, and we use that here.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-interactive.c |  2 ++
 add-interactive.h |  1 +
 add-patch.c       | 21 +++++++++++++++++----
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/add-interactive.c b/add-interactive.c
index e3cc30ad24..bb6acf5ef6 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -61,6 +61,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)
 	FREE_AND_NULL(s->interactive_diff_algorithm);
 	git_config_get_string("diff.algorithm",
 			      &s->interactive_diff_algorithm);
+
+	git_config_get_bool("interactive.singlekey", &s->use_single_key);
 }
 
 void clear_add_i_state(struct add_i_state *s)
diff --git a/add-interactive.h b/add-interactive.h
index 923efaf527..693f125e8e 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -16,6 +16,7 @@ struct add_i_state {
 	char file_old_color[COLOR_MAXLEN];
 	char file_new_color[COLOR_MAXLEN];
 
+	int use_single_key;
 	char *interactive_diff_filter, *interactive_diff_algorithm;
 };
 
diff --git a/add-patch.c b/add-patch.c
index 736bcb4aa7..67741128a8 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -7,6 +7,7 @@
 #include "color.h"
 #include "diff.h"
 #include "sigchain.h"
+#include "compat/terminal.h"
 
 enum prompt_mode_type {
 	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,
@@ -1150,14 +1151,27 @@ static int run_apply_check(struct add_p_state *s,
 	return 0;
 }
 
+static int read_single_character(struct add_p_state *s)
+{
+	if (s->s.use_single_key) {
+		int res = read_key_without_echo(&s->answer);
+		printf("%s\n", res == EOF ? "" : s->answer.buf);
+		return res;
+	}
+
+	if (strbuf_getline(&s->answer, stdin) == EOF)
+		return EOF;
+	strbuf_trim_trailing_newline(&s->answer);
+	return 0;
+}
+
 static int prompt_yesno(struct add_p_state *s, const char *prompt)
 {
 	for (;;) {
 		color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt));
 		fflush(stdout);
-		if (strbuf_getline(&s->answer, stdin) == EOF)
+		if (read_single_character(s) == EOF)
 			return -1;
-		strbuf_trim_trailing_newline(&s->answer);
 		switch (tolower(s->answer.buf[0])) {
 		case 'n': return 0;
 		case 'y': return 1;
@@ -1397,9 +1411,8 @@ static int patch_update_file(struct add_p_state *s,
 			      _(s->mode->prompt_mode[prompt_mode_type]),
 			      s->buf.buf);
 		fflush(stdout);
-		if (strbuf_getline(&s->answer, stdin) == EOF)
+		if (read_single_character(s) == EOF)
 			break;
-		strbuf_trim_trailing_newline(&s->answer);
 
 		if (!s->answer.len)
 			continue;
-- 
gitgitgadget


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox