threads / discuss / 44278

interactive rebase should better highlight the not-applying commit

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

## tl;dr

8 messages between Oct 11, 2016 and Oct 13, 2016.

replies: 7people: 3as markdown or json

Joshua N Pritikin· Oct 11, 2016, 19:07 UTC · lore

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
Stefan Beller· Oct 11, 2016, 20:55 UTC · re: Joshua N Pritikin · lore

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

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?
Johannes Schindelin· Oct 12, 2016, 16:14 UTC · re: Stefan Beller · lore

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

Hi Stefan,
On Tue, 11 Oct 2016, Stefan Beller wrote:
Show 5 quoted lines
> 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

Stefan Beller· Oct 11, 2016, 21:25 UTC · re: Joshua N Pritikin · lore

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

On Tue, Oct 11, 2016 at 12:07 PM, Joshua N Pritikin <jpritikin@pobox.com> wrote:
Show 14 quoted lines
> 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.

Joshua N Pritikin· Oct 12, 2016, 13:27 UTC · re: Stefan Beller · lore

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

On Tue, Oct 11, 2016 at 01:55:22PM -0700, Stefan Beller wrote:
Show 5 quoted lines
> 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:
Show 19 quoted lines
> 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
Johannes Schindelin· Oct 12, 2016, 16:24 UTC · re: Joshua N Pritikin · lore

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

Hi Joshua,
On Wed, 12 Oct 2016, Joshua N Pritikin wrote:
Show 8 quoted lines
> 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.

Show 28 quoted lines
> 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?

Show 5 quoted lines
> 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

Joshua N Pritikin· Oct 12, 2016, 17:02 UTC · re: Johannes Schindelin · lore

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

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?

Show 9 quoted lines
> > 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
Show 7 quoted lines
> > 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?
Show 5 quoted lines
> 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
Johannes Schindelin· Oct 13, 2016, 10:40 UTC · re: Joshua N Pritikin · lore

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

Hi Joshua,
On Wed, 12 Oct 2016, Joshua N Pritikin wrote:
Show 6 quoted lines
> 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 ;-)

Show 11 quoted lines
> > > 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.

Show 16 quoted lines
> > 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.

Show 9 quoted lines
> > 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

← back to recent threads