interactive rebase should better highlight the not-applying commit

interactive rebase should better highlight the not-applying commit

[Joshua N Pritikin]

As of GIT 2.8.1, if you do an interactive rebase and get some conflict 
in the stack of patches then the commit with the conflict is buried in 
4-5 lines of output. It is visually difficult to immediately pick out 
which commit did not apply cleanly. I suggest highlighting the 1 line 
commit summary in red or green or some color to help it stand out from 
all the other output.

I decided to suggest this change after I realized that I probably 
skipped a commit during an interactive rebase instead of resolving the 
conflict. I knew I had to skip some commit so I assumed that I just need 
to skip without reading the commit summary carefully. Now it is 7-15 
days after I did the erroneous rebase. I had to spend a few hours today 
with GIT's archaeology tools to find the lost code.

I assume somebody familiar with GIT's code base could make this change 
in about 10 minutes.

-- 
Joshua N. Pritikin, Ph.D.
Virginia Institute for Psychiatric and Behavioral Genetics
Virginia Commonwealth University
PO Box 980126
800 E Leigh St, Biotech One, Suite 1-133
Richmond, VA 23219
http://people.virginia.edu/~jnp3bc

Re: interactive rebase should better highlight the not-applying commit

[Stefan Beller]

On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> I assume somebody familiar with GIT's code base could make this change
> in about 10 minutes.

Can you elaborate how you come to that estimate?

Re: interactive rebase should better highlight the not-applying commit

[Stefan Beller]

On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> As of GIT 2.8.1, if you do an interactive rebase and get some conflict
> in the stack of patches then the commit with the conflict is buried in
> 4-5 lines of output. It is visually difficult to immediately pick out
> which commit did not apply cleanly. I suggest highlighting the 1 line
> commit summary in red or green or some color to help it stand out from
> all the other output.
>
> I decided to suggest this change after I realized that I probably
> skipped a commit during an interactive rebase instead of resolving the
> conflict. I knew I had to skip some commit so I assumed that I just need
> to skip without reading the commit summary carefully. Now it is 7-15
> days after I did the erroneous rebase. I had to spend a few hours today
> with GIT's archaeology tools to find the lost code.
>

Looking at the actual code, this is not as easy as one might assume,
because rebase is written in shell. (One of the last remaining large commands
in shell), and there is no color support in the die(..) function.

However IIUC currently rebase is completely rewritten/ported to C where it is
easier to add color support as we do have some color support in there already.

Re: interactive rebase should better highlight the not-applying commit

[Joshua N Pritikin]

On Tue, Oct 11, 2016 at 01:55:22PM -0700, Stefan Beller wrote:
> On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > I assume somebody familiar with GIT's code base could make this change
> > in about 10 minutes.
>
> Can you elaborate how you come to that estimate?

Hm, a false belief in the general awesomeness of GIT developers?

On Tue, Oct 11, 2016 at 02:25:19PM -0700, Stefan Beller wrote:
> On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > As of GIT 2.8.1, if you do an interactive rebase and get some conflict
> > in the stack of patches then the commit with the conflict is buried in
> > 4-5 lines of output. It is visually difficult to immediately pick out
> > which commit did not apply cleanly. I suggest highlighting the 1 line
> > commit summary in red or green or some color to help it stand out from
> > all the other output.
> >
> > I decided to suggest this change after I realized that I probably
> > skipped a commit during an interactive rebase instead of resolving the
> > conflict. I knew I had to skip some commit so I assumed that I just need
> > to skip without reading the commit summary carefully. Now it is 7-15
> > days after I did the erroneous rebase. I had to spend a few hours today
> > with GIT's archaeology tools to find the lost code.
> 
> Looking at the actual code, this is not as easy as one might assume, 
> because rebase is written in shell. (One of the last remaining large 
> commands in shell), and there is no color support in the die(..) 
> function.

I'm sorry to hear that.

> However IIUC currently rebase is completely rewritten/ported to C 
> where it is easier to add color support as we do have some color 
> support in there already.

Sounds great. Is there a beta release that I can try out?

Also, I have another wishlist item for (interactive) rebase. Sometimes I 
do a rebase to fix some tiny thing 10-15 commits from HEAD. Maybe only 1 
file is affected and there are no merge conflicts, but when rebase 
reapplies all the commits, the timestamps of lots of unmodified files 
change even though they are unmodified compared to before the rebase. 
Since the modification times are used by 'make' to compute dependencies, 
this creates a lot of useless recompilation that slows things down. It 
would be great if rebase only changed the timestamps of files that were 
actually modified.

Thank you.

-- 
Joshua N. Pritikin, Ph.D.
Virginia Institute for Psychiatric and Behavioral Genetics
Virginia Commonwealth University
PO Box 980126
800 E Leigh St, Biotech One, Suite 1-133
Richmond, VA 23219
http://people.virginia.edu/~jnp3bc

Re: interactive rebase should better highlight the not-applying commit

[Johannes Schindelin]

Hi Stefan,

On Tue, 11 Oct 2016, Stefan Beller wrote:

> On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > I assume somebody familiar with GIT's code base could make this change
> > in about 10 minutes.
> 
> Can you elaborate how you come to that estimate?

Why do you ask? He obviously has "a very good brain" ;-)

Seriously again, Git's source code is not that hard to read, and the Git
developer community is pretty helpful when anybody asks for pointers what
code to change.

Having said that, I did reimplement some parts of the shell script that is
git-rebase--interactive.sh [*1*] in C and am in the process of getting
those integrated into the next (or hopefully not a *much* later) version
of Git.

So what I'd like to see is an *exact* copy-paste of a message in question,
and a *concrete* proposal how it should look like instead.

Ciao,
Johannes

Footnote *1*:
https://github.com/git/git/blob/master/git-rebase--interactive.sh

Re: interactive rebase should better highlight the not-applying commit

[Johannes Schindelin]

Hi Joshua,

On Wed, 12 Oct 2016, Joshua N Pritikin wrote:

> On Tue, Oct 11, 2016 at 01:55:22PM -0700, Stefan Beller wrote:
> > On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > > I assume somebody familiar with GIT's code base could make this
> > > change in about 10 minutes.
> >
> > Can you elaborate how you come to that estimate?
> 
> Hm, a false belief in the general awesomeness of GIT developers?

No, a false belief in your own shortcomings, as you thought it would be
easier to address your wishes for somebody else than you.

> On Tue, Oct 11, 2016 at 02:25:19PM -0700, Stefan Beller wrote:
> > On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > > As of GIT 2.8.1, if you do an interactive rebase and get some conflict
> > > in the stack of patches then the commit with the conflict is buried in
> > > 4-5 lines of output. It is visually difficult to immediately pick out
> > > which commit did not apply cleanly. I suggest highlighting the 1 line
> > > commit summary in red or green or some color to help it stand out from
> > > all the other output.
> > >
> > > I decided to suggest this change after I realized that I probably
> > > skipped a commit during an interactive rebase instead of resolving the
> > > conflict. I knew I had to skip some commit so I assumed that I just need
> > > to skip without reading the commit summary carefully. Now it is 7-15
> > > days after I did the erroneous rebase. I had to spend a few hours today
> > > with GIT's archaeology tools to find the lost code.
> > 
> > Looking at the actual code, this is not as easy as one might assume, 
> > because rebase is written in shell. (One of the last remaining large 
> > commands in shell), and there is no color support in the die(..) 
> > function.
> 
> I'm sorry to hear that.
> 
> > However IIUC currently rebase is completely rewritten/ported to C 
> > where it is easier to add color support as we do have some color 
> > support in there already.
> 
> Sounds great. Is there a beta release that I can try out?

There is no release as such, unless you count Git for Windows v2.10.0.

But you can try the `interactive-rebase` branch of
https://github.com/dscho/git; please note, though, that my main aim was to
be as faithful as possible in the conversion (modulo speed, of course).

> Also, I have another wishlist item for (interactive) rebase.

Hmm. You know, I cannot say that I am a fan of wishlists for Git, unless
the originator of said wishlist takes on their responsibility as an Open
Source user to make their wishes come true.

But maybe I read it all wrong and you do want to make this happen
yourself, and you simply want a little advice how to go about it?

> Sometimes I do a rebase to fix some tiny thing 10-15 commits from HEAD.
> Maybe only 1 file is affected and there are no merge conflicts, but when
> rebase reapplies all the commits, the timestamps of lots of unmodified
> files change even though they are unmodified compared to before the
> rebase.

Well, they *were* modified, right?

A workaround would be to create a new worktree using the awesome `git
worktree` command, perform the rebase there (on an unnamed branch -- AKA
"detached HEAD", no relation to Helloween), and then come back to the
original worktree and reset --hard to the new revision. That reset would
detect that there are actually no changes required to said files.

> Since the modification times are used by 'make' to compute dependencies, 
> this creates a lot of useless recompilation that slows things down. It 
> would be great if rebase only changed the timestamps of files that were 
> actually modified.

Rebase will always have to change those timestamps. Because it really
changes those files. So the mtimes *need* to be updated. As far as rebase
is concerned, it does not matter that the final contents are identical to
*some* previous version...

Ciao,
Johannes

Re: interactive rebase should better highlight the not-applying commit

[Joshua N Pritikin]

On Wed, Oct 12, 2016 at 06:24:37PM +0200, Johannes Schindelin wrote:
> No, a false belief in your own shortcomings, as you thought it would be
> easier to address your wishes for somebody else than you.

Ah, shucks, I guess I could jump in.

> But maybe I read it all wrong and you do want to make this happen
> yourself, and you simply want a little advice how to go about it?

Ugh, if you insist. You really know how to hold someone's feet to the 
fire, eh?

> > On Tue, Oct 11, 2016 at 02:25:19PM -0700, Stefan Beller wrote:
> > > On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > > However IIUC currently rebase is completely rewritten/ported to C 
> > > where it is easier to add color support as we do have some color 
> > > support in there already.
> > 
> > Sounds great. Is there a beta release that I can try out?
> 
> There is no release as such, unless you count Git for Windows v2.10.0.

Nope, that doesn't count. ;-)

> But you can try the `interactive-rebase` branch of
> https://github.com/dscho/git; please note, though, that my main aim was to
> be as faithful as possible in the conversion (modulo speed, of course).

Hm OK

> > Sometimes I do a rebase to fix some tiny thing 10-15 commits from HEAD.
> > Maybe only 1 file is affected and there are no merge conflicts, but when
> > rebase reapplies all the commits, the timestamps of lots of unmodified
> > files change even though they are unmodified compared to before the
> > rebase.
> 
> Well, they *were* modified, right?

Were they? Isn't that just an artefact of the implementation?

> A workaround would be to create a new worktree using the awesome `git
> worktree` command, perform the rebase there (on an unnamed branch -- AKA
> "detached HEAD", no relation to Helloween), and then come back to the
> original worktree and reset --hard to the new revision. That reset would
> detect that there are actually no changes required to said files.

What would be the problem with doing this by default? Or could it be a 
configuration option that can be enabled?

-- 
Joshua N. Pritikin, Ph.D.
Virginia Institute for Psychiatric and Behavioral Genetics
Virginia Commonwealth University
PO Box 980126
800 E Leigh St, Biotech One, Suite 1-133
Richmond, VA 23219
http://people.virginia.edu/~jnp3bc

Re: interactive rebase should better highlight the not-applying commit

[Johannes Schindelin]

Hi Joshua,

On Wed, 12 Oct 2016, Joshua N Pritikin wrote:

> On Wed, Oct 12, 2016 at 06:24:37PM +0200, Johannes Schindelin wrote:
> 
> > But maybe I read it all wrong and you do want to make this happen
> > yourself, and you simply want a little advice how to go about it?
> 
> Ugh, if you insist.

I don't. If you want that feature to see the light of day, you should
insist yourself ;-)

> > > On Tue, Oct 11, 2016 at 02:25:19PM -0700, Stefan Beller wrote:
> > > > On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
> > > > However IIUC currently rebase is completely rewritten/ported to C 
> > > > where it is easier to add color support as we do have some color 
> > > > support in there already.
> > > 
> > > Sounds great. Is there a beta release that I can try out?
> > 
> > There is no release as such, unless you count Git for Windows v2.10.0.
> 
> Nope, that doesn't count. ;-)

Sometimes honesty goes too far. You basically told me that what I work on
does not count. That does not exactly curry my favor.

> > But you can try the `interactive-rebase` branch of
> > https://github.com/dscho/git; please note, though, that my main aim
> > was to be as faithful as possible in the conversion (modulo speed, of
> > course).
> 
> Hm OK
> 
> > > Sometimes I do a rebase to fix some tiny thing 10-15 commits from HEAD.
> > > Maybe only 1 file is affected and there are no merge conflicts, but when
> > > rebase reapplies all the commits, the timestamps of lots of unmodified
> > > files change even though they are unmodified compared to before the
> > > rebase.
> > 
> > Well, they *were* modified, right?
> 
> Were they? Isn't that just an artefact of the implementation?

Yes, they were modified, as the todo script you saved for the interactive
rebase to perform told it to cherry-pick those changes. That is a worktree
operation, performing on files, not a repository operation working on
objects in Git's database.

> > A workaround would be to create a new worktree using the awesome `git
> > worktree` command, perform the rebase there (on an unnamed branch --
> > AKA "detached HEAD", no relation to Helloween), and then come back to
> > the original worktree and reset --hard to the new revision. That reset
> > would detect that there are actually no changes required to said
> > files.
> 
> What would be the problem with doing this by default? Or could it be a
> configuration option that can be enabled?

It could definitely be a new feature that is triggered by a new (opt-in)
configuration option.

It cannot be on by default, at least not in the short run, because those
cherry-picks can fail with merge conflicts and power users of the
interactive rebase expect those conflicts to show in the current worktree.

Ciao,
Johannes